DO NOT MERGE: system table rbr integration branch #97406
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
@ajwerner This is not quite ready. I still need to write a lot of tests. Can you take a look at the overall approach and let me know if you think it needs to change? |
|
Sorry for the slow turnaround. The idea seems good to me. |
This was referenced Mar 2, 2023
Previously, the settingswatcher was one of the last services initialized during sql server start up. Now, it is one of the first services to start up. The settings do not have a well defined value until after settingwatcher initializes. Some settings, like cluster version, must have valid values during initialization. Part of cockroachdb#94843
This commit contains the version and upgrade tasks needed to migrate the system.sql_instances, system.lease, and system.sqlliveness tables to an index format compatible with regional by row tables. Follow on work is needed to add version checks to code interacting with the tables. The migrations will be enabled for each table as the version checks are implemented. The draft PR cockroachdb#97406 contains version checks for each of the tables and was manually tested with this change to ensure the upgrade process works as expected. Part of cockroachdb#98111
This change is intended to support the migration to a regional by row compatible index structure. Encapsulating the feed into a sub object makes it easy to swap the feed implementation when the index changes. Release note: None
- update the bootstrap system schema to use the new index - add version guards for the mgiration - add upgrade tasks to backfill the index and clean up the old index
<what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...>
<what was there before: Previously, ...> <why it needed to change: This was inadequate because ...> <what you did about it: To address this, this patch ...>
jeffswenson
force-pushed
the
jeffswenson-sqlliveness-rbr-migration
branch
from
c100bde to
77654bd
Compare
jeffswenson
added a commit
to jeffswenson/cockroach
that referenced
this pull request
Mar 8, 2023
This commit contains the version and upgrade tasks needed to migrate the system.sql_instances, system.lease, and system.sqlliveness tables to an index format compatible with regional by row tables. Follow on work is needed to add version checks to code interacting with the tables. The migrations will be enabled for each table as the version checks are implemented. The draft PR cockroachdb#97406 contains version checks for each of the tables and was manually tested with this change to ensure the upgrade process works as expected. Part of cockroachdb#98111
jeffswenson
added a commit
to jeffswenson/cockroach
that referenced
this pull request
Mar 9, 2023
This commit contains the version and upgrade tasks needed to migrate the system.sql_instances, system.lease, and system.sqlliveness tables to an index format compatible with regional by row tables. Follow on work is needed to add version checks to code interacting with the tables. The migrations will be enabled for each table as the version checks are implemented. The draft PR cockroachdb#97406 contains version checks for each of the tables and was manually tested with this change to ensure the upgrade process works as expected. Part of cockroachdb#98111
craig bot
pushed a commit
that referenced
this pull request
Mar 9, 2023
98044: kv: async resolve unreplicated locks on other ranges after one-phase commit r=nvanbenschoten a=nvanbenschoten Fixes #94400. This commit fixes async lock resolution on external ranges after a transaction exercises the one-phase commit fast-path. Before the change, the 1PC fast-path was failing to handle the case where a transaction had acquired locks on ranges other than the range with the transaction record. As a result, these locks were not asynchronously but eagerly cleaned up. Conflicting transactions that encountered these locks would then wait 50ms before pushing the lock holder, finding it to be committed, and removing the locks themselves. It's fairly rare for a transaction to acquire locks on multiple ranges but then still be able to perform a 1PC commit. Regardless, we should handle such cases. Release note (bug fix): Fixed a bug where transactions that performed a SELECT FOR UPDATE across multiple ranges but never performed writes could fail to eagerly clean up their locks after commit. Future transactions that encountered these abandoned locks could be delayed for 50ms before unlocking them. `@kaisun314` I'm adding you as an optional reviewer here because this PR extends some of the new testing that you added for intent resolution. 98174: changefeedccl: Fix test flake r=miretskiy a=miretskiy Do not trigger flow registry drain prematurely. Fixes #97233 Release note: None 98186: upgrade: add system rbr upgrade and versions r=JeffSwenson a=JeffSwenson This commit contains the version and upgrade tasks needed to migrate the system.sql_instances, system.lease, and system.sqlliveness tables to an index format compatible with regional by row tables. Follow on work is needed to add version checks to code interacting with the tables. The migrations will be enabled for each table as the version checks are implemented. The draft PR #97406 contains version checks for each of the tables and was manually tested with this change to ensure the upgrade process works as expected. Part of #98111 98279: ccl/sqlproxyccl: do not classify TCP probes as a connection error r=pjtatlow,JeffSwenson a=jaylim-crl #### ccl/sqlproxyccl: do not classify TCP probes as a connection error Previously, if a client attempted to open a TCP connection without any packets sent, the proxy will log an error (i.e. while receiving startup message), and close the connection. This scenario is common with TCP probes, and this is what cloud load balancers generally do. To avoid these probes spamming the proxy logs, we will silently ignore these class of errors, and this commit does that. #### ccl/sqlproxyccl: add proxy.sql.accepted_conns metric to the proxy This commit adds a new proxy.sql.accepted_conns metric to indicate the number of accepted connections by the proxy. The previous commit removed logging messages for connections that do not have any data packets. This new metric would aid in debugging (e.g. high cpu usage). Release note: None Epic: none Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Yevgeniy Miretskiy <[email protected]> Co-authored-by: Jeff <[email protected]> Co-authored-by: Jay <[email protected]>
jeffswenson
changed the title
jeffswenson
changed the title
|
Closing this PR now that all of the changes are merged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.