Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: improve TestTxnObeysTableModificationTime #23681

Closed
wants to merge 1 commit into from

Conversation

vivekmenezes
Copy link
Contributor

removes the use of SNAPSHOT ISOLATION in the test.

Tests read-only, write-only, and read-write transaction
scenarios.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

t.Fatal(err)
}

// Insert an entry so that the transaction is guaranteed to be
// assigned a timestamp.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is no longer needed because transaction timestamps are assigned during BEGIN

t.Fatal(err)
}

if err := tx.Commit(); !testutils.IsError(err, "transaction deadline exceeded") {
if err := txWrite.Commit(); !testutils.IsError(err, "TransactionStatusError: transaction deadline exceeded") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good example of why this ought to be retryable

@vivekmenezes vivekmenezes force-pushed the insertalter branch 3 times, most recently from 8a97976 to beef95a Compare March 9, 2018 21:20
removes the use of SNAPSHOT ISOLATION in the test.

Tests read-only, write-only, and read-write transaction
scenarios.

Release note: None
@bdarnell
Copy link
Member

:lgtm:

Since @vivekmenezes is out of the office I'm going to incorporate this into #27040.


Review status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale)


Comments from Reviewable

@andreimatei
Copy link
Contributor

@vivekmenezes there were requests for more docs in #27040, if you're so inclined.
Around https://reviewable.io/reviews/cockroachdb/cockroach/27040#-LG77SmB3YfgDt8vVYdm

@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label Jun 19, 2019
@jordanlewis jordanlewis closed this May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants