Uh oh!
There was an error while loading. Please reload this page.
HDDS-1119. DN get OM certificate from SCM CA for block token validat… - #601
Conversation
hadoop-yetus
commented
Mar 13, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 13, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 13, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 13, 2019
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Javadoc copy paste error like above, this one set and does not return.
Uh oh!
There was an error while loading. Please reload this page.
02166c1 to
2b742fdComparehadoop-yetus
commented
Mar 14, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 14, 2019
💔 -1 overall
This message was automatically generated. |
a015644 to
0b57d76Comparehadoop-yetus
commented
Mar 14, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 14, 2019
💔 -1 overall
This message was automatically generated. |
e44820d to
02392daComparehadoop-yetus
commented
Mar 14, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 14, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 15, 2019
💔 -1 overall
This message was automatically generated. |
hadoop-yetus
commented
Mar 15, 2019
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Should we loadAllCertificates() again only if the map does not contain it?
Also, in the contsructor when we call loadAllCertificates(), should we asset the passing in certSerialId is loaded into the map from file system?
There was a problem hiding this comment.
initialized local certificate during initial call to loadAllCertificates. Now if it is null at L202 than it is also not present in map, so loading it again from filesystem and checking it again is the only option.
There was a problem hiding this comment.
We can assert in constructor if certSerialId is not null than its corresponding certificate should not be null as well after loading from memory but if we fail than it will become catch 22 for some recovery scenarios. (Ex we can't even create instance of CertificateClient to call init which may handle some of the automatic recovery in future)
There was a problem hiding this comment.
Let's discuss and file follow up JIRAs. I'm OK with the current approach as-is.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
why do we need to change s3secret manager here from interface to impl? seems not relate to this ticket.
There was a problem hiding this comment.
S3SecretManagerImpl has helper function to return OMMetadataManager required in secret store. Its interface doesn't. Since it is returning a dependency i think its better to not push it to interface itself.
xiaoyuyao
commented
Mar 15, 2019
Thanks @ajayydv for the update. We are almost there. Just few last issues as commented inline. |
hadoop-yetus
commented
Mar 15, 2019
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
commented
Mar 15, 2019
Thanks @ajayydv for the update. +1 the latest push, pending fix of the related findbugs issue and unit test results. |
hadoop-yetus
commented
Mar 15, 2019
💔 -1 overall
This message was automatically generated. |
bb81529 to
9f59f08Comparehadoop-yetus
commented
Mar 15, 2019
hadoop-yetus
commented
Mar 16, 2019
💔 -1 overall
This message was automatically generated. |
…ion. Contributed by Ajay Kumar.
hadoop-yetus
commented
Mar 19, 2019
💔 -1 overall
This message was automatically generated. |
ajayydv
commented
Mar 19, 2019
@xiaoyuyao thanks for continuous reviews of this long patch. UT failures looks unrelated. |
- Fix getSystemStreamMetadata in InMemoryManager to filter based on the system name on top of stream names Author: Bharath Kumarasubramanian <bkumaras@linkedin.com> Reviewers: Xinyu Liu <xinyuiscool@github.com> Closesapache#601 from bharathkk/in-memory-fix
…ion. Contributed by Ajay Kumar.