[
https://issues.apache.org/jira/browse/HDDS-1101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16768976#comment-16768976
]
Anu Engineer commented on HDDS-1101:
------------------------------------
[~xyao] Thanks for the review, v1 fixes some of the issues. Please see below
for details.
bq. DefaultApprover.java#Line 104: is there a reason to use
Time.monotonicNowNanos() as the serial ID for the certificate? This may be OK
for a single SCM case. But the ID may collide when there are multiple SCM
instances. Should reserve certain bits to partition the scm ids?
The Serial ID is a BigInteger, so there is no limit the number of bits can
have. So it is easy to add an ID field if we needed. When we do the HA work, we
can easily add this.
bq. DefaultCAServer.java#Line 213: should we store after
xcertHolder.complete(xcert);?
I also debated this. Here are the two options we have
1. Store and then return - In case of failure we store the certificate, but the
client might not see it.
2. Complete and then Store - In a case where we have flagged the complete and
for some reason we fail to store, the client will get a certificate which is
not persisted. While I fully believe that will not happen in real life, it felt
the first path was the easier path to understand, hence I picked that path. I
am willing to do the second if you feel so.
bq. Line 245-250: should we wrap this with supplyAsync to make the revoke truly
async?
Yes, but we need to wrap this just like the function above when we support
human approved revoke.
Since we are not supporting it, for now, I have just written minimum needed
code for now.
bq. StorageContainerManager.java#Line 266: NIT: typo "afte" should be "after"
Fixed.
bq. Line 268: question wrt. the configurator usage: why don't we populate the
value initialized back into the configurator with the setters or just assume
only the injector will set it?
I see where you are going with this, we can set it back in the injector and the
user can get these values back.
We might want to do that in the future, right now all the fields that are set
have corresponding getxxxx functions in the StorageContainerManager class. But
it would be useful if and when we support more internal fields.
bq. Line 531: should we move the certStore down to internal of DefaultCAServer?
I eventually want to move DefaultCAserver to Hadoop-common, that way we can
support a certificate infrastructure for Hadoop itself. The Impl. class is in
scm-server class and has dependencies on things like RocksDB. I wanted to avoid
that so it is easy to move into Hadoop common later.
bq. TestOmMultiPartKeyInfoCodec.java#Line 57: NIT: typo: random
Fixed.
> SCM CA: Write Certificate information to SCM Metadata
> -----------------------------------------------------
>
> Key: HDDS-1101
> URL: https://issues.apache.org/jira/browse/HDDS-1101
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Components: SCM
> Reporter: Anu Engineer
> Assignee: Anu Engineer
> Priority: Major
> Attachments: HDDS-1101.000.patch, HDDS-1101.001.patch
>
>
> Make SCM CA write to the Metadata layer of SCM.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
