[ 
https://issues.apache.org/jira/browse/HDFS-16771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17604946#comment-17604946
 ] 

ASF GitHub Bot commented on HDFS-16771:
---------------------------------------

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?





> JN should tersely print logs about NewerTxnIdException
> ------------------------------------------------------
>
>                 Key: HDFS-16771
>                 URL: https://issues.apache.org/jira/browse/HDFS-16771
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: ZanderXu
>            Assignee: ZanderXu
>            Priority: Major
>              Labels: pull-request-available
>
> JournalNode should tersely print some logs about NewerTxnIdException.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to