You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/09/14 20:05:34 UTC
[GitHub] [hadoop] xkrogen commented on a diff in pull request #4882: HDFS-16771. JN should tersely print logs about NewerTxnIdException
xkrogen commented on code in PR #4882:
URL: https://github.com/apache/hadoop/pull/4882#discussion_r971240283
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/client/QuorumJournalManager.java:
##########
@@ -524,9 +524,6 @@ public void selectInputStreams(Collection<EditLogInputStream> streams,
selectRpcInputStreams(rpcStreams, fromTxnId, onlyDurableTxns);
streams.addAll(rpcStreams);
return;
- } catch (NewerTxnIdException ntie) {
- // normal situation, we requested newer IDs than any journal has. no new streams
- return;
Review Comment:
So it seems that this logic was never working? I guess that means the tests added in #4560 aren't working properly, we probably need to also confirm that in the "normal" case (where `sinceTxId == highestTxId + 1`), `selectStreamingInputStreams` is never called.
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/JournalNodeRpcServer.java:
##########
@@ -114,6 +114,7 @@ public class JournalNodeRpcServer implements QJournalProtocol,
.setVerbose(false)
.build();
+ this.server.addTerseExceptions(NewerTxnIdException.class);
Review Comment:
good catch, we should probably add `CacheMissException` as well, WDYT?
##########
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/Journal.java:
##########
@@ -751,7 +751,11 @@ public GetJournaledEditsResponseProto getJournaledEdits(long sinceTxId,
"it via " + DFSConfigKeys.DFS_HA_TAILEDITS_INPROGRESS_KEY);
}
long highestTxId = getHighestWrittenTxId();
- if (sinceTxId > highestTxId) {
+ if (sinceTxId == highestTxId + 1) {
+ // Requested edits that don't exist yet; short-circuit the cache here
+ metrics.rpcEmptyResponses.incr();
+ return GetJournaledEditsResponseProto.newBuilder().setTxnCount(0).build();
+ } else if (sinceTxId > highestTxId + 1) {
// Requested edits that don't exist yet and is newer than highestTxId.
Review Comment:
Can you add some more detail in the two comments here to make it more clear why we treat the `+ 1` case differently?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org