xkrogen commented on PR #4744:
URL: https://github.com/apache/hadoop/pull/4744#issuecomment-1226624906

   > I'm sorry, I just find this comment, but didn't find related code to 
finalize the previous inProgress segment. Can you share the related code? 
Thanks.
   
   I'm referring to this:
   
https://github.com/apache/hadoop/blob/62c86eaa0e539a4307ca794e0fcd502a77ebceb8/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/qjournal/server/Journal.java#L574-L583
   
   But a little more digging made me realize that I don't think what I 
described will actually happen, since in `FSEditLog#openForWrite()` before 
calling `startLogSegment()` we first check that there are no active streams:
   
https://github.com/apache/hadoop/blob/63db1a85e376c2266afdc62b9590e40acc98429c/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java#L338-L347
   
   So it will actually throw an exception, rather than finalizing the old 
segment as I said previously. But this is _after_ `catchupDuringFailver()`, so 
to make your original proposal (disable in-progress edits) work properly, we 
still need to modify `recoverUnclosedStreams()` to throw an error when it fails 
instead of just swallowing the exception.
   
   I briefly looked at the other usages of `recoverUnclosedStreams()` and I 
don't really see any reason why we would want to swallow the exception ... The 
TODO comment there is also from 2012, 10 years old now :)
   
   So are we agreed that the best way forward is to modify 
`recoverUnclosedStreams()` to throw exception on failure, then we can use 
`inProgressOk = false` to solve this problem as you originally proposed?


-- 
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]

Reply via email to