sql,kv: replace InitPuts with CPuts #137808
Draft
+455
−349
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 |
yuzefovich
force-pushed
the
init-put-removal
branch
2 times, most recently
from
cb96693 to
17f610d
Compare
yuzefovich
changed the title
CPut with empty `expValue` parameter is the same as InitPut (with `failOnTombstones=false`), so we replace the latter with the former throughout the code base. This paves the way to deprecating the InitPut request altogether. Release note: None
yuzefovich
force-pushed
the
init-put-removal
branch
from
17f610d to
5b61e9a
Compare
This commit extends the CPut command with FailOnTombstones option that we already have in the InitPut - it makes it so that the CPut fails with a ConditionFailedError if it encounters a tombstone. This begins the process of folding all usages of InitPuts into CPuts. (We actually have only one place where we use InitPuts with this flag set - in the sql liveness system which allows us to ensure that after a session is dead, it cannot be resurrected, which allows us to keep the dead session cache easily.) InitPut request was introduced long time ago in 1a95630 when we considered extending CPut with a boolean for special tombstone handling too burdensome and ugly. Since then we have been extending the CPut with a few options, so adding an extra one doesn't seem like a big deal anymore. On the plus side, it removes the confusion that I personally had for why we used InitPut as opposed to CPut with nil expected value in a few places (all were removed in the previous commit). The deprecation was alluded to during the review of cockroachdb#35157. In terms of deprecating the InitPut altogether here is the expected timeline: - sometime after compatibility with 25.1 is no longer needed (so 25.3+), we will remove the last spot were we still issue InitPut in the sql liveness system - sometime after compatibilty with the version mentioned above (say 26.1+), we'll be able to remove all InitPut-related code. Release note: None
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.
sql: replace InitPut with CPut
CPut with empty
expValueparameter is the same as InitPut (withfailOnTombstones=false), so we replace the latter with the former throughout the code base. This paves the way to deprecating the InitPut request altogether.kv: extend CPut with FailOnTombstones option
This commit extends the CPut command with FailOnTombstones option that we already have in the InitPut - it makes it so that the CPut fails with a ConditionFailedError if it encounters a tombstone. This begins the process of folding all usages of InitPuts into CPuts. (We actually have only one place where we use InitPuts with this flag set - in the sql liveness system which allows us to ensure that after a session is dead, it cannot be resurrected, which allows us to keep the dead session cache easily.)
InitPut request was introduced long time ago in 1a95630 when we considered extending CPut with a boolean for special tombstone handling too burdensome and ugly. Since then we have been extending the CPut with a few options, so adding an extra one doesn't seem like a big deal anymore. On the plus side, it removes the confusion that I personally had for why we used InitPut as opposed to CPut with nil expected value in a few places (all were removed in the previous commit).
The deprecation was alluded to during the review of #35157.
In terms of deprecating the InitPut altogether here is the expected timeline:
TODO: can we do it faster?
Release note: None
Epic: None