Skip to content

[Enhancement] (image) check image validity as soon as generated #9011

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.

Sign up for GitHub

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

Merged
lide-reed merged 6 commits into from
Apr 25, 2022
Merged

[Enhancement] (image) check image validity as soon as generated #9011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
706625d
load newly generated image file as soon as generated to check if it i…
Henry2SS Apr 13, 2022
51ad74c
delete the latest invalid image file
Henry2SS Apr 13, 2022
b34c3ac
fix
Henry2SS Apr 13, 2022
3431f24
fix
Henry2SS Apr 14, 2022
5248293
get filePath from saveImage() to ensure deleting the correct file whi…
Henry2SS Apr 21, 2022
33c21ff
fix
Henry2SS Apr 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/catalog/Catalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1950,7 +1950,8 @@ public long loadSqlBlockRule(DataInputStream in, long checksum) throws IOExcepti
}

// Only called by checkpoint thread
public void saveImage() throws IOException {
// return the latest image file's absolute path
public String saveImage() throws IOException {
// Write image.ckpt
Storage storage = new Storage(this.imageDir);
File curFile = storage.getImageFile(replayedJournalId.get());
Expand All @@ -1963,6 +1964,7 @@ public void saveImage() throws IOException {
curFile.delete();
throw new IOException();
}
return curFile.getAbsolutePath();
}

public void saveImage(File curFile, long replayedJournalId) throws IOException {
Expand Down
34 changes: 33 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/master/Checkpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import com.google.common.base.Strings;

import java.io.IOException;
import java.io.OutputStream;
import java.net.HttpURLConnection;
Expand Down Expand Up @@ -111,6 +113,8 @@ public synchronized void doCheckpoint() {
catalog = Catalog.getCurrentCatalog();
catalog.setEditLog(editLog);
createStaticFieldForCkpt();
boolean exceptionCaught = false;
String latestImageFilePath = null;
try {
catalog.loadImage(imageDir);
catalog.replayJournal(checkPointVersion);
Expand All @@ -119,13 +123,25 @@ public synchronized void doCheckpoint() {
checkPointVersion, catalog.getReplayedJournalId()));
}
catalog.fixBugAfterMetadataReplayed(false);
catalog.saveImage();
latestImageFilePath = catalog.saveImage();
replayedJournalId = catalog.getReplayedJournalId();

// destroy checkpoint catalog, reclaim memory
catalog = null;
Catalog.destroyCheckpoint();
destroyStaticFieldForCkpt();

// Load image to verify if the newly generated image file is valid
// If success, do all the following jobs
// If failed, just return
catalog = Catalog.getCurrentCatalog();
catalog.loadImage(imageDir);
if (MetricRepo.isInit) {
MetricRepo.COUNTER_IMAGE_WRITE_SUCCESS.increase(1L);
}
LOG.info("checkpoint finished save image.{}", replayedJournalId);
} catch (Throwable e) {
exceptionCaught = true;
e.printStackTrace();
LOG.error("Exception when generate new image file", e);
if (MetricRepo.isInit) {
Expand All @@ -137,6 +153,22 @@ public synchronized void doCheckpoint() {
catalog = null;
Catalog.destroyCheckpoint();
destroyStaticFieldForCkpt();
// if new image generated && exception caught, delete the latest image here
// delete the newest image file, cuz it is invalid
if ((!Strings.isNullOrEmpty(latestImageFilePath)) && exceptionCaught) {
MetaCleaner cleaner = new MetaCleaner(Config.meta_dir + "/image");
try {
cleaner.cleanTheLatestInvalidImageFile(latestImageFilePath);
if (MetricRepo.isInit) {
MetricRepo.COUNTER_IMAGE_CLEAN_SUCCESS.increase(1L);
}
} catch (Throwable ex) {
LOG.error("Master delete latest invalid image file failed.", ex);
if (MetricRepo.isInit) {
MetricRepo.COUNTER_IMAGE_CLEAN_FAILED.increase(1L);
}
}
}
}

// push image file to all the other non master nodes
Expand Down
11 changes: 11 additions & 0 deletions fe/fe-core/src/main/java/org/apache/doris/persist/MetaCleaner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,17 @@ public void clean() throws IOException {
}
}
}

public void cleanTheLatestInvalidImageFile(String path) throws IOException {
File latestInvalidImage = new File(path);
if (latestInvalidImage.exists()) {
if (latestInvalidImage.delete()) {
LOG.info(latestInvalidImage.getAbsoluteFile() + " deleted.");
} else {
LOG.warn(latestInvalidImage.getAbsoluteFile() + " delete failed.");
Copy link
Contributor

@lide-reed lide-reed Apr 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it print the reason to the warning log if delete failed

Copy link
Contributor Author

@Henry2SS Henry2SS Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/apache/incubator-doris/blob/87fc46f84c81c197607bccfa68056209ba226f6c/fe/fe-core/src/main/java/org/apache/doris/master/Checkpoint.java#L161-L166

It will print the exception to the warning log as error.

}
}
}

private String fileType(File file) throws IOException {
String type = null;
Expand Down