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.

Sign up for GitHub

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

Jump to bottom

Use conditional mutations for AccumuloStore. Add checks for status and putRepo #4160

Merged
merged 11 commits into from
Jan 30, 2024
Merged

Use conditional mutations for AccumuloStore. Add checks for status and putRepo #4160

merged 11 commits into from
Jan 30, 2024

Conversation

DomGarguilo
Copy link
Member

Closes #4114

@DomGarguilo DomGarguilo self-assigned this Jan 12, 2024
@DomGarguilo DomGarguilo changed the title Use conditional mutations for AccumuloStore Use conditional mutations for AccumuloStore. Add checks for status and putRepo Jan 19, 2024
@DomGarguilo DomGarguilo marked this pull request as ready for review January 19, 2024 19:16
@DomGarguilo
Copy link
Member Author

Marked this PR as ready for review. Leaving other checks to be done in follow on PRs.

In this PR:

  • Converted FateMutatorImpl to use ConditionalMutations
  • Added a method (FateMutatorImpl.requireStatus) add a condition that checks the status value is what we expect and use it in AccumuloStore.create(). Created a new Iterator to make this possible
  • Added a condition to putRepo() to make sure the spot we are inserting into is empty.
  • Added a new IT to test the two mentioned added checks.

DomGarguilo and others added 3 commits January 22, 2024 10:12
@DomGarguilo
Use batch writer when there are no conditions
d8a3a05
@DomGarguilo
Merge remote-tracking branch 'upstream/elasticity' into accStoreCondi…
3bbefef 
…tionalMutations

# Conflicts:
#	core/src/main/java/org/apache/accumulo/core/fate/accumulo/AccumuloStore.java
#	core/src/main/java/org/apache/accumulo/core/fate/accumulo/FateMutatorImpl.java
@DomGarguilo @keith-turner
Add retry to create method and add test
9240cad 
Co-authored-by: Keith Turner <[email protected]>
keith-turner
keith-turner approved these changes Jan 29, 2024
View reviewed changes
test/src/main/java/org/apache/accumulo/test/fate/accumulo/AccumuloStoreIT.java
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class AccumuloStoreIT extends SharedMiniClusterBase {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nice test. If possible would be nice to eventually run the test against Zookeeper also like #4202. If it makes sense could be a follow on.

core/src/main/java/org/apache/accumulo/core/fate/accumulo/StatusMappingIterator.java Show resolved Hide resolved
core/src/main/java/org/apache/accumulo/core/fate/accumulo/StatusMappingIterator.java Outdated Show resolved Hide resolved
test/src/main/java/org/apache/accumulo/test/fate/accumulo/FateMutatorImplIT.java Show resolved Hide resolved
@DomGarguilo DomGarguilo merged commit 11949de into apache:elasticity Jan 30, 2024
8 checks passed
@DomGarguilo DomGarguilo deleted the accStoreConditionalMutations branch January 30, 2024 15:03
@DomGarguilo DomGarguilo mentioned this pull request Feb 8, 2024
Closed
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keith-turner keith-turner keith-turner approved these changes

@cshannon cshannon Awaiting requested review from cshannon

Assignees

@DomGarguilo DomGarguilo

Labels
None yet
Projects
Accumulo: Elasticity
Status: ✅ Done
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@DomGarguilo @keith-turner @ctubbsii