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

Backport FateInterleavingIT to 3.1 #5223

Open
wants to merge 1 commit into
base: 3.1
Choose a base branch
from

Conversation

dlmarion
Copy link
Contributor

@dlmarion dlmarion commented Jan 3, 2025

No description provided.

@dlmarion dlmarion added this to the 3.1.0 milestone Jan 3, 2025
@dlmarion dlmarion self-assigned this Jan 3, 2025
Copy link
Member

@kevinrr888 kevinrr888 left a comment

Choose a reason for hiding this comment

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

Briefly looked through this and it looks good. Should maybe wait to merge this until we figure out what to do with #5214. I would also like to do a more thorough review later

Comment on lines +147 to +152
store.reserve(txid);
try {
status = store.getStatus(txid);
} finally {
store.unreserve(txid, Duration.ZERO);
}
Copy link
Member

Choose a reason for hiding this comment

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

is reserving required in order to get the status in 3.x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@Override
public TStatus getStatus(long tid) {
verifyReserved(tid);
return _getStatus(tid);
}

Comment on lines +217 to +218
assertEquals(2, subset.keySet().stream()
.filter(k -> k.getColumnFamily().toString().equals("TXID:" + txid)).count());
Copy link
Member

Choose a reason for hiding this comment

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

For this and similar assertions, could also ensure that the isReady call occurs before the call call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants