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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
