[
https://issues.apache.org/jira/browse/HDDS-120?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16678693#comment-16678693
]
Xiaoyu Yao commented on HDDS-120:
---------------------------------
Thanks [~dineshchitlangia] for working on this. Patch v2 looks pretty good to
me overall. Here are a few minor comments:
*BlockData.java*
Line 272-275: can be simplified with .append(blockID.toString())
Line 285-291: this is not necessary as castChunkList() already has similar
logic.
Line 297: can we bump up the log level to debug?
*ContainerCommandRequestPBHelper.java*
Line 43: should we use switch against msg.getCmdType() instead of the
hard-coded CmdType number?
This way, the code is more readable and will not need to be changed when
protobuf changes.
Line 45: NIT: can we have a local variable like below before the switch
statement and use it in the cases?
String containerID = String.valueOf(msg.getContainerID()?
Can we add containerType into auditParams for CreateContainer?
Line 53: can we add forceUdpate into auditParams for UpdateContainer?
Line 71: NIT: we might get a lot of chunk info for large file, which might not
be desirable.
Maybe we should have a option to disable audit log chunk info by default.
Line 155: given this is for put small file, I think it should be OK to include
the chunkInfo here in the audit log.
Line 180: can we add a debug log for unknown cmd type?
*HddsDispatcher.java*
Line 71/72: can we make this a enum?
Line 81: we may not need dnActionMap, DNAction class can provide a method to
parse the enum of CmdType and convert it to DNAction directly.
Line 125: we should avoid using hardcoded enum numbers.
Line 132: can you elaborate on the usage of shouldAuditFailure and the logic
around 194-200?
I see it is set as false but line 145/153/163 still audit log failures.
Currently, only the createContainer is audited. Do you intend to add others in
later patches?
*Ozone*
Line 71: NIT: can we rename HDFS_DN_OPTS to HDDS_DN_OPTS?
> Adding HDDS datanode Audit Log
> ------------------------------
>
> Key: HDDS-120
> URL: https://issues.apache.org/jira/browse/HDDS-120
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Xiaoyu Yao
> Assignee: Dinesh Chitlangia
> Priority: Major
> Labels: alpha2
> Attachments: HDDS-120.001.patch, HDDS-120.002.patch
>
>
> This can be useful to find users who overload the DNs.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
