Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

[Transactions3/ABR] Need to abort at the put timestamp or higher #5847

Merged
merged 4 commits into from
Jan 11, 2022

Conversation

jeremyk-91
Copy link
Contributor

Goals (and why):

  • Abort values correctly in Transactions3 for the cleanup task

Implementation Description (bullets):

  • Aborts didn't seem to be happening at a high enough value.

Testing (What was existing testing like? What have you done to improve it?):

Concerns (what feedback would you like?):
to investigate: this looks like it's set lower down, why doesn't it work already?

Where should we start reviewing?: small

Priority (whenever / two weeks / yesterday): this week please, blocks transactions3 and ABR rollouts!

@jeremyk-91 jeremyk-91 requested a review from gmaretic January 11, 2022 12:24
@changelog-app
Copy link

changelog-app bot commented Jan 11, 2022

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

We now abort values in Transactions3 for purposes of cleaning the transactions range correctly when restoring from backup. Normal users are not affected.

Check the box to generate changelog(s)

  • Generate changelog entry

@@ -123,7 +124,6 @@ public Statement bindAbortStatement(PreparedStatement preparedAbortStatement, Tr
BoundStatement bound = preparedAbortStatement.bind(rowKeyBb, columnNameBb, valueBb);
return bound.setConsistencyLevel(ConsistencyLevel.QUORUM)
.setSerialConsistencyLevel(ConsistencyLevel.SERIAL)
.setDefaultTimestamp(CellValuePutter.SET_TIMESTAMP + 1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From javadoc of Statement#setDefaultTimestamp(long):

   * <p>Note that unlike other configuration, when this statement is prepared {@link
   * BoundStatement}s created off of {@link PreparedStatement} do not inherit this configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't seem like this should apply, given that we set it on the bound statement already

Copy link
Contributor

@gmaretic gmaretic left a comment

Choose a reason for hiding this comment

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

I certainly hope this fixes it

@jeremyk-91
Copy link
Contributor Author

also need to handle STAGING (can be separate pr)

@gmaretic
Copy link
Contributor

It already works with STAGING?

@bulldozer-bot bulldozer-bot bot merged commit 39cb1a0 into develop Jan 11, 2022
@bulldozer-bot bulldozer-bot bot deleted the jkong/tx3-the-other-side branch January 11, 2022 13:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants