xkrogen commented on PR #4744: URL: https://github.com/apache/hadoop/pull/4744#issuecomment-1220981198
> If the active crashed, during standby starting active services, the standby will recover unclosed streams via `recoverUnclosedStreams`. So before `catchupDuringFailover`, the last segment should always closed. I don't think this will always be true. In `recoverUnclosedStreams`, if the finalization fails, it will just ignore it and assume that it will be handled later (by `Journal#startLogSegment()`, which will automatically close an old stream when you try to open a new one). https://github.com/apache/hadoop/blob/63db1a85e376c2266afdc62b9590e40acc98429c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java#L1660-L1670 So if we want to disable in-progress edits during `catchupDuringFailover`, then we have to modify this to retry/abort when the old segments cannot be closed. But I don't think this is the right approach. I think you might have also understood what I was saying in my last comment here: > Perhaps we should add a way for callers of QuorumJournalManager to indicate that they want to use the streaming mechanism, as opposed to RPC. Disabling in-progress edits achieves this, but is too strong (note that the streaming mechanism can also load in-progress edits). Looking at the current diff, I don't see a reason to add `LogsPurgeable#enableInProgressTailing()`. We already have the `inProgressOk` parameter of `selectInputStreams()`, so why do we need a different mechanism to adjust whether we use in-progress edits? What I was trying to say is that what we really want to do, both in this Jira and in HDFS-14806, is _disable the RPC mechanism and use the streaming mechanism_. In HDFS-14806, to do this, we turned off in-progress edits, which was okay. But in this Jira, it's not okay -- we don't want the RPC mechanism, but we _still need in-progress edits_. So basically I am saying we need something like the `inProgressOk` parameter, but for RPC vs. streaming. I am thinking we can add a new parameter to `LogsPurgeable#selectInputStreams()` like `preferBulkReads`. RPC vs streaming is an implementation detail of QJM, but "bulk reads" could be general. If this parameter is true, then QJM can prefer the streaming mechanism (other JournalManagers can ignore it). Callers like `BootstrapStandby#checkLogsAvailableForRead`, `BackupImage#tryConvergeJournalSpool`, `FSImage#loadFSImage`, and of course `EditLogTailer#catchupDuringFailover()` can pass `preferBulkReads = true`. -- 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]
