tomscut commented on a change in pull request #4082:
URL: https://github.com/apache/hadoop/pull/4082#discussion_r830454966
##########
File path:
hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java
##########
@@ -1509,13 +1509,18 @@ synchronized void abortCurrentLogSegment() {
* effect.
*/
@Override
- public synchronized void purgeLogsOlderThan(final long minTxIdToKeep) {
+ public synchronized void purgeLogsOlderThan(long minTxIdToKeep) {
// Should not purge logs unless they are open for write.
// This prevents the SBN from purging logs on shared storage, for example.
if (!isOpenForWrite()) {
return;
}
-
+
+ // Reset purgeLogsFrom to avoid purging edit log which is in progress.
+ if (isSegmentOpen()) {
+ minTxIdToKeep = minTxIdToKeep > curSegmentTxId ? curSegmentTxId :
minTxIdToKeep;
Review comment:
> Just below this we have:
>
> ```java
> assert curSegmentTxId == HdfsServerConstants.INVALID_TXID || // on
format this is no-op
> minTxIdToKeep <= curSegmentTxId :
> "cannot purge logs older than txid " + minTxIdToKeep +
> " when current segment starts at " + curSegmentTxId;
> ```
>
> We assert that `minTxIdToKeep <= curSegmentTxId`. So in the situation you
described where `minTxIdToKeep > curSegmentTxId`, shouldn't the assert fail?
Thank you @xkrogen very much for your reply.
We did add assertion. But in the production, assertions are usually disabled
and do not actually take effect. I think we should add strict logic to avoid
this problem. What do you think of this?
--
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-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
