From: Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org, yaroslav.dynnikov@tarantool.org Subject: Re: [Tarantool-patches] [PATCH 2/2] vshard: fix buckets_count() on replicas Date: Thu, 13 May 2021 23:23:26 +0300 [thread overview] Message-ID: <00948536-25b1-dcf2-abc7-6e22d9a683e2@tarantool.org> (raw) In-Reply-To: <04d7d05f09a5ee7ed52b27c480e81232c406e415.1620903962.git.v.shpilevoy@tarantool.org> Hi! Thanks for your patch! Three comments below. On 13.05.2021 14:07, Vladislav Shpilevoy wrote: > +local function schema_install_triggers() > + local _bucket = box.space._bucket > + if M.bucket_on_replace then > + local ok, err = pcall(_bucket.on_replace, _bucket, nil, > + M.bucket_on_replace) > + if not ok then > + log.warn('Could not drop old trigger from '.. > + '_bucket: %s', err) > + end > + end > + _bucket:on_replace(bucket_generation_increment) > + M.bucket_on_replace = bucket_generation_increment > +end > + > +local function schema_install_on_replace(_, new) > + -- Wait not just for _bucket to appear, but for the entire > + -- schema. This might be important if the schema will ever > + -- consist of more than just _bucket. > + if new[1] ~= 'vshard_version' then Is it possible that someone drops something from _schema space? I mean that in such case new will be nil. I could provide one such case - when user needs to execute box.once once again, (s)he drops corresponding record from _schema space. > + return > + end > + schema_install_triggers() > + > + local _schema = box.space._schema nit: you could get it from the third argument of the trigger. But up to you. > + local ok, err = pcall(_schema.on_replace, _schema, nil, M.schema_on_replace) > + if not ok then > + log.warn('Could not drop trigger from _schema inside of the '.. > + 'trigger: %s', err) > + end > + M.schema_on_replace = nil > + -- Drop the caches which might have been created while the > + -- schema was being replicated. > + bucket_generation_increment() > +end > + > +-- > +-- Install the triggers later when there is an actual schema to install them on. > +-- On replicas it might happen that they are vshard-configured earlier than the > +-- master and therefore don't have the schema right away. > +-- > +local function schema_install_triggers_delayed() > + log.info('Could not find _bucket space to install triggers - delayed '.. > + 'until the schema is replicated') > + assert(not box.space._bucket) > + local _schema = box.space._schema > + if M.schema_on_replace then > + local ok, err = pcall(_schema.on_replace, _schema, nil, > + M.schema_on_replace) > + if not ok then > + log.warn('Could not drop trigger from _schema: %s', err) > + end > + end > + _schema:on_replace(schema_install_on_replace) > + M.schema_on_replace = schema_install_on_replace > +end > + > -- VShard versioning works in 4 numbers: major, minor, patch, and > -- a last helper number incremented on every schema change, if > -- first 3 numbers stay not changed. That happens when users take > @@ -2612,17 +2676,15 @@ local function storage_cfg(cfg, this_replica_uuid, is_reload) > local uri = luri.parse(this_replica.uri) > schema_upgrade(is_master, uri.login, uri.password) > > - if M.bucket_on_replace then > - box.space._bucket:on_replace(nil, M.bucket_on_replace) > - M.bucket_on_replace = nil > - end > - lref.cfg() > - if is_master then > - box.space._bucket:on_replace(bucket_generation_increment) > - M.bucket_on_replace = bucket_generation_increment > + if is_master or box.space._bucket then Is it possible that "is_master" is true and _bucket is nil? Why simple `if box.space._bucket` is not enough?
next prev parent reply other threads:[~2021-05-13 20:24 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-13 11:07 [Tarantool-patches] [PATCH 0/2] vshard.storage.buckets_count() bug Vladislav Shpilevoy via Tarantool-patches 2021-05-13 11:07 ` [Tarantool-patches] [PATCH 1/2] test: fix test output on latest Tarantool Vladislav Shpilevoy via Tarantool-patches 2021-05-13 20:23 ` Oleg Babin via Tarantool-patches 2021-05-19 21:50 ` Yaroslav Dynnikov via Tarantool-patches 2021-05-21 19:26 ` Vladislav Shpilevoy via Tarantool-patches 2021-05-21 21:54 ` Yaroslav Dynnikov via Tarantool-patches 2021-05-13 11:07 ` [Tarantool-patches] [PATCH 2/2] vshard: fix buckets_count() on replicas Vladislav Shpilevoy via Tarantool-patches 2021-05-13 20:23 ` Oleg Babin via Tarantool-patches [this message] 2021-05-21 19:26 ` Vladislav Shpilevoy via Tarantool-patches 2021-05-24 6:57 ` Oleg Babin via Tarantool-patches 2021-05-19 21:51 ` Yaroslav Dynnikov via Tarantool-patches 2021-05-21 19:30 ` Vladislav Shpilevoy via Tarantool-patches 2021-05-13 20:23 ` [Tarantool-patches] [PATCH 0/2] vshard.storage.buckets_count() bug Oleg Babin via Tarantool-patches 2021-05-25 20:43 ` Vladislav Shpilevoy via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=00948536-25b1-dcf2-abc7-6e22d9a683e2@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=olegrok@tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --cc=yaroslav.dynnikov@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 2/2] vshard: fix buckets_count() on replicas' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox