[
https://issues.apache.org/jira/browse/HDFS-10519?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15383298#comment-15383298
]
Andrew Wang commented on HDFS-10519:
------------------------------------
Hi Jiayi, thanks for working on this. I took a look at the patch and have
mostly style comments. It looks pretty good overall, appreciate the new tests.
* config key should mention "edit log" somewhere. We can reuse an existing
prefix and call it something like "dfs.ha.tail-edits.in-progress"
* Would be good to explain a little more in hdfs-default.xml why a user might
want to turn this on.
* EditLogTailer, regarding the comment: edits themselves are not in-progress,
an edit log segment is. Consider renaming the boolean also.
* "isTail" is also not very descriptive, what it's really doing is bounding the
tailing to the committed txID length right? Better to name it accordingly,
rather than tie it to a single usecase like standby tailing.
Journal.java
* inProgress boolean is unused. Conceptually the JNs shouldn't have to be aware
of the standby doing in-progress tailing anyway.
* getCommittedTxnIdForTests isn't used only in tests, rename it?
* Can you add a comment on the {{numTxns == 0}} early return, explaining why we
do this?
* JournalSet, why is the correct committed txnid to set here {{0}} rather than
the max txId from the set of logs?
QuorumOutputStream
* Let's pull the boolean out of the conf and pass it into the constructor
rather than pass in the entire Configuration, this helps limit the scope. The
boolean should also be named something more descriptive, like
"updateCommittedTxnId".
* The new conditional dummy flush also needs a comment for sure. How stale
would the committed txnId be if we didn't write this 0-len segment? Any ideas
on how to further optimize this (e.g. make it more async)?
* QuorumJournalManager, the ternary can be written instead with Math.min yea?
I'd prefer to see a little logic rework that avoids the need for the {{else}}
also.
RemoteEditLogManifest
* Do we need the single-arg form of the constructor? We only use it once in a
test, maybe just always require the two-arg form.
* Initialize committedTxnId to some invalid txn id, and validate it in
checkState? Should also be within bounds.
* toString output should included the committedTxnId
* TestBKSR, TestFJM, you don't need to modify the call since we kept the old
overload.
* TestNNSRManager, we should pass through the parameter in the mock, like the
other args
TestStandbyInProgressTail
* Typo: edig
* Typo: shoudl
* Do we need the Thread.sleep for tailing? Can we trigger edit log tailing
manually instead? Sleeps are unreliable and increase test runtime.
* Looks like some of the startup/shutdown logic is shared, can we use @Before
and @After annotated methods to share code? As an FYI we also normally do a
null check on cluster before calling cluster.shutdown, as an extra guard.
* New assert methods can be made static
* Any thoughts on hooking this up to the randomized fault-injection tests
originally developed for QJM? I think we'd all have more confidence in this
feature if we ran the fuzzer for a while and it came back clean.
> Add a configuration option to enable in-progress edit log tailing
> -----------------------------------------------------------------
>
> Key: HDFS-10519
> URL: https://issues.apache.org/jira/browse/HDFS-10519
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: ha
> Reporter: Jiayi Zhou
> Assignee: Jiayi Zhou
> Priority: Minor
> Attachments: HDFS-10519.001.patch, HDFS-10519.002.patch,
> HDFS-10519.003.patch, HDFS-10519.004.patch, HDFS-10519.005.patch,
> HDFS-10519.006.patch, HDFS-10519.007.patch
>
>
> Standby Namenode has the option to do in-progress edit log tailing to improve
> the data freshness. In-progress tailing is already implemented, but it's not
> enabled as default configuration. And there's no related configuration key to
> turn it on.
> Adding a related configuration key to let Standby Namenode is reasonable and
> would be a basis for further improvement on Standby Namenode.
--
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
