-
Notifications
You must be signed in to change notification settings - Fork 594
HDDS-13861. Intermittent failure in testOverWriteKeyWithAndWithOutVersioning [2/2] #9261
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
Conversation
…thOutVersioning (apache#9234)" This reverts commit 1fa7e56.
…e purging of deleted table entries
…tingService in versioning test
adoroszlai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @peterxcli for digging the root cause of this failure.
| OMMetadataManager metadataManager = cluster.getOzoneManager().getMetadataManager(); | ||
| String ozoneKey = metadataManager.getOzoneKey(volumeName, bucketName, keyName); | ||
|
|
||
| OmKeyInfo omKeyInfo = metadataManager.getKeyTable(getBucketLayout()).get(ozoneKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The part of the change related to getBucketLayout() should not be reverted, because this test uses a different layout:
Lines 4608 to 4609 in c6d7bd1
| private BucketLayout getBucketLayout() { | |
| return BucketLayout.DEFAULT; |
vs.
Lines 4623 to 4627 in c6d7bd1
| // This test inspects RocksDB delete table to check for versioning | |
| // information. This is easier to do with object store keys. | |
| volume.createBucket(bucketName, BucketArgs.newBuilder() | |
| .setVersioning(versioning) | |
| .setBucketLayout(BucketLayout.OBJECT_STORE).build()); |
To simplify things, maybe we should add a constant referencing OBJECT_STORE, and use it in both createRequiredForVersioningTest and checkExceptedResultForVersioningTest.
| assertThat(rangeKVs).isNotEmpty(); | ||
| assertThat(rangeKVs.size()).isGreaterThan(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also keep isNotEmpty().
Co-authored-by: Doroszlai, Attila <adoroszlai@apache.org>
f65790c to
995adbd
Compare
|
thanks @peterxcli ! |
|
Thanks @adoroszlai @rich7420 for reviewing! |
What changes were proposed in this pull request?
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13861
How was this patch tested?