-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-17540: Do not propagate ExecutionException and add maxRetries limit to NameNode edek cache warmup #6774
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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
simbadzina
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.
I'm concerned will changing the contract of warmUpEncryptedKeys from
exception if any key isn't initialized
to
exception only if all keys can't be initialized
You can still like the number of retries, without changing the above.
...-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
Outdated
Show resolved
Hide resolved
.../hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java
Show resolved
Hide resolved
.../hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java
Outdated
Show resolved
Hide resolved
...oop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirEncryptionZoneOp.java
Show resolved
Hide resolved
...-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/ValueQueue.java
Outdated
Show resolved
Hide resolved
.../hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirEncryptionZoneOp.java
Show resolved
Hide resolved
8574577 to
77d14b7
Compare
...-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestValueQueue.java
Show resolved
Hide resolved
...-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestValueQueue.java
Outdated
Show resolved
Hide resolved
simbadzina
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.
LGTM.
|
Could you please to carve out the changes in hadoop-common into a separate PR. I'm suspecting this may be what is causing the CI tests to hang and fail. Besides the CI tests, keeping the changes contained within a module is cleaner for maintainability and modularity. |
154e46e to
87a4d12
Compare
…mit to NameNode edek cache warmup
|
🎊 +1 overall
This message was automatically generated. |
simbadzina
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.
LGTM
…mit to NameNode edek cache warmup (apache#6774)
…mit to NameNode edek cache warmup (apache#6774)
Description of PR
Depends on PR #6860
JIRA = HDFS-17540
The ekek cache warm up thread should not fail the whole warmup of other keys if an invalid key is encountered.
We have observed infinite retries to KMS if one of Encryption Key is not available.
Change it to
How was this patch tested?
Added unit test TestFSDirEncryptionZoneOp for retry behavior
Related unit tests
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles? NA