kv: fix cluster_logical_timestamp() #41438
Merged
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 |
nvanbenschoten
approved these changes
Oct 8, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @nvanbenschoten)
pkg/kv/txn_test.go, line 634 at r1 (raw file):
commitTS := txn.CommitTimestamp() // We expect to have refreshed just after the timestamp injected by the error. expTS := refreshTS.Add(0, 1)
The race detector is ok with this, right?
This was referenced Oct 8, 2019
bdarnell
approved these changes
Oct 8, 2019
Before this patch, the txn.CommitTimestamp() function (which powers the cluster_logical_timestamp(), among others) was failing to take into consideration possible refreshes that might have happened since the current epoch began (i.e. txn.RefreshedTimestamp). Fixes cockroachdb#41424 Release note (bug fix): Fix a bug causing the cluster_logical_timestamp() function to sometimes return incorrect results.
andreimatei
force-pushed
the
kv.fix-cluster-logical-timestamp
branch
from
f899de6
to
b491dde
Compare
andreimatei
commented
Oct 8, 2019
There was a problem hiding this comment.
bors r+
Reviewable status:
andreimatei
commented
Oct 8, 2019
There was a problem hiding this comment.
Reviewable status:
pkg/kv/txn_test.go, line 634 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
The race detector is ok with this, right?
yup
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Oct 8, 2019
41323: sql: bevy of fixes for ActiveRecord compatibility r=jordanlewis a=jordanlewis - improve efficiency of `'foo'::REGCLASS` - improve efficiency of `pg_get_coldesc` - parse INTERVAL(6) as a no-op meaning INTERVAL - bugfix to generate_subscripts for oidvector and int2vector - add pg_available_extensions - fix `pg_get_indexdef` and `pg_attrdef.adbin` to be more Postgres compatible With these fixes, 90% of the fork in `activerecord-cockroachdb-adapter` can go away. 41438: kv: fix cluster_logical_timestamp() r=andreimatei a=andreimatei Before this patch, the txn.CommitTimestamp() function (which powers the cluster_logical_timestamp(), among others) was failing to take into consideration possible refreshes that might have happened since the current epoch began (i.e. txn.RefreshedTimestamp). Fixes #41424 Release note (bug fix): Fix a bug causing the cluster_logical_timestamp() function to sometimes return incorrect results. Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com> Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Build succeeded |
craig bot
pushed a commit
that referenced
this pull request
Oct 8, 2019
41439: release-19.2: kv: fix cluster_logical_timestamp() r=andreimatei a=andreimatei Backport 1/1 commits from #41438. /cc @cockroachdb/release --- Before this patch, the txn.CommitTimestamp() function (which powers the cluster_logical_timestamp(), among others) was failing to take into consideration possible refreshes that might have happened since the current epoch began (i.e. txn.RefreshedTimestamp). Fixes #41424 Release note (bug fix): Fix a bug causing the cluster_logical_timestamp() function to sometimes return incorrect results. Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
craig bot
pushed a commit
that referenced
this pull request
Oct 9, 2019
41457: sql: (revert the reversion) fix various problematic uses of the txn in DistSQL flows r=andreimatei a=andreimatei This is a reversion of #41406, which was a reversion of #41304 #41304 had been revert because it exposed an existing bug more broadly. That bug was fixed by #41438. So here we go again with this patch. See individual commits for what the patch does. Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
craig bot
pushed a commit
that referenced
this pull request
Oct 9, 2019
41470: release-19.2: sql: (revert the reversion) fix various problematic uses of the txn in DistSQL flows r=andreimatei a=andreimatei Backport 4/4 commits from #41457. /cc @cockroachdb/release --- This is a reversion of #41406, which was a reversion of #41304 #41304 had been revert because it exposed an existing bug more broadly. That bug was fixed by #41438. So here we go again with this patch. See individual commits for what the patch does. Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
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.
Before this patch, the txn.CommitTimestamp() function (which powers
the cluster_logical_timestamp(), among others) was failing to take into
consideration possible refreshes that might have happened since the
current epoch began (i.e. txn.RefreshedTimestamp).
Fixes #41424
Release note (bug fix): Fix a bug causing the
cluster_logical_timestamp() function to sometimes return incorrect
results.