[tm_state] updateLocked should re-populate local metadata tables to reflect promotion rule changes #209
Conversation
…eflect promotion rule changes Signed-off-by: Andrew Mason <amason@slack-corp.com> Defer populating metadata to handle different queryservice states If we're enabling the queryservice, wait until afterwards to populate the metadata. On the other hand, if we're disabling the queryservice, then populate the metadata just before shutting down the queryservice. Signed-off-by: Andrew Mason <amason@slack-corp.com> Update testTM mysqldaemon to support local_metadata queries Signed-off-by: Andrew Mason <amason@slack-corp.com> Add missing `updateLocked` call on tablettype change in test Signed-off-by: Andrew Mason <amason@slack-corp.com> Track whether tm_state is opening, and try to respect the `init_populate_metadata` flag Signed-off-by: Andrew Mason <amason@slack-corp.com> Separate metadata table creation from upserting Signed-off-by: Andrew Mason <amason@slack-corp.com> Add a MetadataManager to replace the TabletManager.LocalMetadataPopulator func Also, track if we've previously called PopulateMetadataTables, and if so, only do the Upsert rather than Create+Upsert Signed-off-by: Andrew Mason <amason@slack-corp.com>
| localMetadata := ts.tm.getLocalMetadataValues(ts.tablet.Type) | ||
| dbName := topoproto.TabletDbName(ts.tablet) | ||
|
|
||
| if !ts.hasCreatedMetadataTables { |
There was a problem hiding this comment.
It is not obvious to me why this check is needed. It seems that with the initial populatemetadata in tm_init this check should't be required.
There was a problem hiding this comment.
Hah, I agree. It took me a while to convince myself I had this correct:
- In tm_init, during
handleRestore:- if
-restore_from_backupwas set, then we populate (and create) the metadata tables as a side effect - if
-init_populate_metadatawas set, then we populate (and create) the metadata tables explicitly - if neither flag was set, the metadata tables do not get created
- if
This check is there to cover that third case, where neither flag was passed to the vttablet binary. This was discussed a bit in vitessio#5783, in particular:
Note that we have chosen not to change the behavior of metadata_tables if
-init_populate_metadatais false in the presence of-restore_from_backupbecause doing so would break backwards compatibility.
Maybe now is the right time to revisit that, and break that older behavior to simplify this.
There was a problem hiding this comment.
Oh I see. That makes sense. Maybe we should add a comment about this. Because it's not an obvious insight .
One more nit - I would set the following inside PopulateMetadataTables:
ts.hasCreatedMetadataTables = true
This cherry-picks a squashed version of vitessio#8107 into our master branch
Signed-off-by: Andrew Mason amason@slack-corp.com
Defer populating metadata to handle different queryservice states
If we're enabling the queryservice, wait until afterwards to populate
the metadata. On the other hand, if we're disabling the queryservice,
then populate the metadata just before shutting down the queryservice.
Signed-off-by: Andrew Mason amason@slack-corp.com
Update testTM mysqldaemon to support local_metadata queries
Signed-off-by: Andrew Mason amason@slack-corp.com
Add missing
updateLockedcall on tablettype change in testSigned-off-by: Andrew Mason amason@slack-corp.com
Track whether tm_state is opening, and try to respect the
init_populate_metadataflagSigned-off-by: Andrew Mason amason@slack-corp.com
Separate metadata table creation from upserting
Signed-off-by: Andrew Mason amason@slack-corp.com
Add a MetadataManager to replace the TabletManager.LocalMetadataPopulator func
Also, track if we've previously called PopulateMetadataTables, and if
so, only do the Upsert rather than Create+Upsert
Signed-off-by: Andrew Mason amason@slack-corp.com
Description
Related Issue(s)
Checklist
Deployment Notes
Impacted Areas in Vitess
Components that this PR will affect: