Skip to content

release-22.2: sql: add assertion for txn passed to internal executor#88772

Merged
ZhouXing19 merged 2 commits intorelease-22.2from
blathers/backport-release-22.2-88295
Oct 13, 2022
Merged

release-22.2: sql: add assertion for txn passed to internal executor#88772
ZhouXing19 merged 2 commits intorelease-22.2from
blathers/backport-release-22.2-88295

Conversation

@blathers-crl
Copy link
Copy Markdown

@blathers-crl blathers-crl Bot commented Sep 26, 2022

Backport 2/2 commits from #88295 on behalf of @ZhouXing19.

/cc @cockroachdb/release


If an internal executor is created with a txn binding to it, it should not be used to execute a statement with a nil txn.

Release note: None


Release justification: Bug fixes and low-risk updates to new functionality

@blathers-crl blathers-crl Bot requested a review from a team September 26, 2022 21:25
@blathers-crl blathers-crl Bot force-pushed the blathers/backport-release-22.2-88295 branch from 5a8e11c to fd2acd9 Compare September 26, 2022 21:25
@blathers-crl
Copy link
Copy Markdown
Author

blathers-crl Bot commented Sep 26, 2022

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Patches should only be created for serious issues or test-only changes.
  • Patches should not break backwards-compatibility.
  • Patches should change as little code as possible.
  • Patches should not change on-disk formats or node communication protocols.
  • Patches should not add new functionality.
  • Patches must not add, edit, or otherwise modify cluster versions; or add version gates.
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

  • What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
  • Will this work in a cluster of mixed patch versions? Did we test that?
  • If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

@blathers-crl blathers-crl Bot requested a review from ajwerner September 26, 2022 21:25
@blathers-crl blathers-crl Bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Sep 26, 2022
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ZhouXing19)

@ZhouXing19 ZhouXing19 force-pushed the blathers/backport-release-22.2-88295 branch from fd2acd9 to b30f7ab Compare September 27, 2022 14:54
Previously, the internal executor always run queries with a nil txn, even though
it's constructed with a txn binding to it.

We now change it to pass the txn to the internal executor in
`findNextTableToUpgrade()`

Release note: None
If a internal executor is created with a txn binding to it, it should not be
used to execute statement with a nil txn.

Release note: None
@ZhouXing19 ZhouXing19 force-pushed the blathers/backport-release-22.2-88295 branch from b30f7ab to ec51315 Compare October 6, 2022 14:27
@ZhouXing19 ZhouXing19 merged commit 97c79d2 into release-22.2 Oct 13, 2022
@ZhouXing19 ZhouXing19 deleted the blathers/backport-release-22.2-88295 branch October 13, 2022 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants