[
https://issues.apache.org/jira/browse/HDFS-10924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15556560#comment-15556560
]
Xiao Chen commented on HDFS-10924:
----------------------------------
Thanks [~jingcheng...@intel.com] for the nice work here, and on the related
HDFS-9668.
Some comments on patch 3, looks good overall:
{{AutoCloseableReadWriteLockWrapper}}:
- Are the constructors of {{AutoCloseableReadLock}} and
{{AutoCloseableWriteLock}} intentionally public? I'm having a hard time coming
up with a scenario of this.
- Maybe we could update the above to constructors to accept the {{lock}}
parameter. This is also more consistent with what the new instrumented lock
class does. (as well as java's {{ReentrantReadWriteLock}} class)
{{InstrumentedAutoCloseableReadWriteLockWrapper}}:
- Please check input variables on
{{InstrumentedAutoCloseableReadWriteLockWrapper}}'s constructor. (e.g. time >
0, logger not null. Do we allow null name?)
- Personally feels the following change would be more readable:
{code}
if (lockWarningThresholdMs - lockHeldTime < 0) {
...
now = clock.monotonicNow();
localLastLogTs = lastLogTimestamp.get();
long deltaSinceLastLog = now - localLastLogTs;
// check should print log or not
if (deltaSinceLastLog - minLoggingGapMs < 0) {
warningsSuppressed.incrementAndGet();
return;
}
{code}
to
{code}
if (lockHeldTime > lockWarningThresholdMs) {
....
now = clock.monotonicNow();
localLastLogTs = lastLogTimestamp.get();
if (now - localLastLogTs < minLoggingGapMs) {
warningsSuppressed.incrementAndGet();
return;
}
{code}
- Feels like a StringBuilder would be more readable and efficient in
{{logWarning}}, than the current 3 concatenation.
> Add a new instrumented read-write lock
> --------------------------------------
>
> Key: HDFS-10924
> URL: https://issues.apache.org/jira/browse/HDFS-10924
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: datanode
> Reporter: Jingcheng Du
> Assignee: Jingcheng Du
> Attachments: HDFS-10924-2.patch, HDFS-10924-3.patch, HDFS-10924.patch
>
>
> Add a new instrumented read-write lock in hadoop common, so that the
> HDFS-9668 can use this to improve the locking in FsDatasetImpl
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
