-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ALLUXIO-2408] Fix potential journal checkpoint failure #4306
Changes from 2 commits
ad66225
e33873f
b50235d
7b5c785
2ed0c2e
deb1ee1
c0a9908
d731c30
04b690f
bf2bf4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
/* | ||
* The Alluxio Open Foundation licenses this work under the Apache License, version 2.0 | ||
* (the "License"). You may not use this work except in compliance with the License, which is | ||
* available at www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, | ||
* either express or implied, as more fully set forth in the License. | ||
* | ||
* See the NOTICE file distributed with this work for information regarding copyright ownership. | ||
*/ | ||
|
||
package alluxio.master.journal; | ||
|
||
import alluxio.Constants; | ||
import alluxio.underfs.UnderFileSystem; | ||
import alluxio.util.UnderFileSystemUtils; | ||
|
||
import com.google.common.base.Preconditions; | ||
import com.google.common.base.Throwables; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.io.IOException; | ||
|
||
/** | ||
* A class which manages the checkpoint for a journal. The {@link #updateCheckpoint(String)} method | ||
* will update the journal's checkpoint file to a specified checkpoint, and | ||
* {@link recoverCheckpoint()} will recover from any failures that may occur during | ||
* {@link #updateCheckpoint(String)}. | ||
* | ||
* The checkpoint updating process goes | ||
* <pre> | ||
* 1. Write a new checkpoint named checkpoint.data.tmp | ||
* 2. Rename checkpoint.data to checkpoint.data.backup.tmp | ||
* 3. Rename checkpoint.data.backup.tmp to checkpoint.data.backup | ||
* 4. Rename checkpoint.data.tmp to checkpoint.data | ||
* 5. Delete completed logs | ||
* 6. Delete checkpoint.data.backup | ||
* </pre> | ||
*/ | ||
public final class CheckpointManager { | ||
private static final Logger LOG = LoggerFactory.getLogger(Constants.LOGGER_TYPE); | ||
|
||
/** The UFS where the journal is being written to. */ | ||
private final UnderFileSystem mUfs; | ||
/** | ||
* Absolute path to the checkpoint file. This is where the latest checkpoint is stored. During | ||
* normal operation (when not writing a new checkpoint file), this is the only checkpoint file | ||
* that exists. If this file exists and there is no {@link #mTempBackupCheckpointPath}, it plus | ||
* all completed logs, plus the active log, should represent the full state of the master. | ||
*/ | ||
private final String mCheckpointPath; | ||
/** | ||
* Absolute path to the backup checkpoint file. The latest checkpoint is saved here while renaming | ||
* the temporary checkpoint so that we can recover in case the rename fails. If this file and | ||
* {@link #mCheckpointPath} both exist, {@link #mCheckpointPath} is the most up to date checkpoint | ||
* and {@link #mBackupCheckpointPath} should be deleted. | ||
*/ | ||
private final String mBackupCheckpointPath; | ||
/** | ||
* Absolute path to the temporary backup checkpoint file. This path is used as an intermediate | ||
* rename step when backing up {@link #mCheckpointPath} to {@link #mBackupCheckpointPath}. As long | ||
* as this file exists, it supercedes mCheckpointPath as the most up to date checkpoint file. | ||
*/ | ||
private final String mTempBackupCheckpointPath; | ||
/** | ||
* A journal writer through which this checkpoint manager can delete completed logs when the | ||
* checkpoint is updated. | ||
*/ | ||
private final JournalWriter mWriter; | ||
|
||
/** | ||
* Creates a new instance of {@link CheckpointManager}. | ||
* | ||
* @param ufs the under file system holding the journal | ||
* @param the directory for the journal | ||
*/ | ||
public CheckpointManager(UnderFileSystem ufs, String checkpointPath, JournalWriter writer) { | ||
mUfs = ufs; | ||
mCheckpointPath = checkpointPath; | ||
mBackupCheckpointPath = mCheckpointPath + ".backup"; | ||
mTempBackupCheckpointPath = mBackupCheckpointPath + ".tmp"; | ||
mWriter = writer; | ||
} | ||
|
||
/** | ||
* Recovers the checkpoint file in case the master crashed while updating it previously. | ||
*/ | ||
public void recoverCheckpoint() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you document the expected state after this method is called? ie. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
try { | ||
Preconditions.checkState( | ||
!(mUfs.exists(mCheckpointPath) && mUfs.exists(mTempBackupCheckpointPath) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will there be duplicate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we cannot guarantee atomicity between exists & next operation, I think we should just cache the values for the duration of the recovery, unless we expect an update. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
&& mUfs.exists(mBackupCheckpointPath)), | ||
"checkpoint, temp backup checkpoint, and backup checkpoint should never exist " | ||
+ "simultaneously"); | ||
if (mUfs.exists(mTempBackupCheckpointPath)) { | ||
// If mCheckpointPath exists, step 2 must have implemented rename as copy + delete, and | ||
// failed during the delete. | ||
UnderFileSystemUtils.deleteIfExists(mUfs, mCheckpointPath); | ||
mUfs.rename(mTempBackupCheckpointPath, mCheckpointPath); | ||
} | ||
if (mUfs.exists(mBackupCheckpointPath)) { | ||
// We must have crashed after step 3 | ||
if (mUfs.exists(mCheckpointPath)) { | ||
// We crashed after step 4, so we can finish steps 5 and 6. | ||
mWriter.deleteCompletedLogs(); | ||
mUfs.delete(mBackupCheckpointPath, false); | ||
} else { | ||
// We crashed before step 4, so we roll back to backup checkpoint. | ||
mUfs.rename(mBackupCheckpointPath, mCheckpointPath); | ||
} | ||
} | ||
} catch (IOException e) { | ||
throw Throwables.propagate(e); | ||
} | ||
} | ||
|
||
/** | ||
* Updates the checkpoint file to the checkpoint at the specified path. The update is done in such | ||
* a way that {@link #recoverCheckpoint()} will recover from any failures that occur during the | ||
* update. | ||
* | ||
* @param newCheckpointPath the path to the new checkpoint file | ||
*/ | ||
public void updateCheckpoint(String newCheckpointPath) { | ||
try { | ||
if (mUfs.exists(mCheckpointPath)) { | ||
UnderFileSystemUtils.deleteIfExists(mUfs, mTempBackupCheckpointPath); | ||
UnderFileSystemUtils.deleteIfExists(mUfs, mBackupCheckpointPath); | ||
// Rename in two steps so that we never have identical mCheckpointPath and | ||
// mBackupCheckpointPath. This is a concern since UFS may implement rename as copy + delete. | ||
mUfs.rename(mCheckpointPath, mTempBackupCheckpointPath); | ||
mUfs.rename(mTempBackupCheckpointPath, mBackupCheckpointPath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT: maybe add a log message after this that the backup of the checkpoint was completed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
} | ||
mUfs.rename(newCheckpointPath, mCheckpointPath); | ||
LOG.info("Renamed checkpoint file {} to {}", newCheckpointPath, mCheckpointPath); | ||
|
||
// The checkpoint already reflects the information in the completed logs. | ||
mWriter.deleteCompletedLogs(); | ||
UnderFileSystemUtils.deleteIfExists(mUfs, mBackupCheckpointPath); | ||
} catch (IOException e) { | ||
throw Throwables.propagate(e); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: the javadoc params seem out of date with the actual parameters of the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed