[Feature] Support Cluster Snapshot Backup: SQL Interface and meta data (part 1) #54447
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…a (part 1) Signed-off-by: srlch <[email protected]>
![starrocks-cr[bot]](https://avatars.githubusercontent.com/in/404334?s=60&v=4){kind=small}
| public void write(DataOutput out) throws IOException { | ||
| Text.writeString(out, GsonUtils.GSON.toJson(this)); | ||
| } | ||
| } |
There was a problem hiding this comment.
The most risky bug in this code is:
Typo in the method call createAutomatedSnaphot, which should be createAutomatedSnapshot.
You can modify the code like this:
private void createSnapshotIfJobIsFinished() {
GlobalStateMgr.getCurrentState().getClusterSnapshotMgr().createAutomatedSnapshot(this);
}
| @Override | ||
| public void gsonPostProcess() throws IOException { | ||
| } | ||
| } |
There was a problem hiding this comment.
The most risky bug in this code is:
In the load method, the deserialized object data is not used to update the state of the current ClusterSnapshotMgr. This means that the data read from the SRMetaBlockReader, which presumably represents a saved state, does nothing to restore or set the internal state of the object using it.
You can modify the code like this:
public void load(SRMetaBlockReader reader)
throws SRMetaBlockEOFException, IOException, SRMetaBlockException {
ClusterSnapshotMgr data = reader.readJson(ClusterSnapshotMgr.class);
this.automatedSnapshotSvName = data.automatedSnapshotSvName;
this.automatedSnapshot = data.automatedSnapshot;
this.historyAutomatedSnapshotJobs = data.historyAutomatedSnapshotJobs;
this.remoteStorage = data.remoteStorage;
this.locationWithServiceId = data.locationWithServiceId;
}
| String json = GsonUtils.GSON.toJson(this); | ||
| Text.writeString(out, json); | ||
| } | ||
| } |
There was a problem hiding this comment.
The most risky bug in this code is:
The read method may fail if the input JSON does not match the expected structure or if required fields are missing or null.
You can modify the code like this:
public static ClusterSnapshotLog read(DataInput in) throws IOException {
try {
String json = Text.readString(in);
ClusterSnapshotLog log = GsonUtils.GSON.fromJson(json, ClusterSnapshotLog.class);
// Add checks to ensure required fields are present and valid
if (log.type == null) {
throw new IOException("ClusterSnapshotLog type cannot be null");
}
return log;
} catch (Exception e) {
throw new IOException("Failed to read ClusterSnapshotLog from input", e);
}
}
Comment on lines
738
to
740
| adminSetAutomatedSnapshotStatement | ||
| : ADMIN SET AUTOMATED CLUSTER SNAPSHOT (ON (STORAGE VOLUME svName=identifier)? | OFF) | ||
| ; |
There was a problem hiding this comment.
Suggested change
| adminSetAutomatedSnapshotStatement | |
| : ADMIN SET AUTOMATED CLUSTER SNAPSHOT (ON (STORAGE VOLUME svName=identifier)? | OFF) | |
| ; | |
| adminSetAutomatedSnapshotOnStatement | |
| : ADMIN SET AUTOMATED CLUSTER SNAPSHOT ON (STORAGE VOLUME svName=identifier)? | |
| ; | |
| adminSetAutomatedSnapshotOffStatement | |
| : ADMIN SET AUTOMATED CLUSTER SNAPSHOT OFF | |
| ; |
Signed-off-by: srlch <[email protected]>
|
|
||
| public ClusterSnapshotMgr() {} | ||
|
|
||
| public void addAutomatedSnapshotRequest(String storageVolumeName, boolean isReplay) { |
There was a problem hiding this comment.
Suggested change
| public void addAutomatedSnapshotRequest(String storageVolumeName, boolean isReplay) { | |
| // Turn on automated snapshot, use stmt for extension in future | |
| public void setAutomatedSnapshotOn(AdminSetAutomatedSnapshotOnStmt stmt) { |
| return getAutomatedSnapshot() != null; | ||
| } | ||
|
|
||
| public boolean containAutomatedSnapshotRequest() { |
There was a problem hiding this comment.
Suggested change
| public boolean containAutomatedSnapshotRequest() { | |
| public boolean isAutomatedSnapshotOn() { |
| return snapshot.getSnapshotName(); | ||
| } | ||
|
|
||
| public void dropAutomatedSnapshotRequest(boolean isReplay) { |
There was a problem hiding this comment.
Suggested change
| public void dropAutomatedSnapshotRequest(boolean isReplay) { | |
| // Turn off automated snapshot, use stmt for extension in future | |
| public void setAutomatedSnapshotOff(AdminSetAutomatedSnapshotOffStmt stmt) { |
Comment on lines
190
to
193
| @Override | ||
| public void write(DataOutput out) throws IOException { | ||
| Text.writeString(out, GsonUtils.GSON.toJson(this)); | ||
| } |
| @SerializedName(value = "automatedSnapshot") | ||
| private ClusterSnapshot automatedSnapshot = null; | ||
| @SerializedName(value = "historyAutomatedSnapshotJobs") | ||
| private List<ClusterSnapshotJob> historyAutomatedSnapshotJobs = Lists.newArrayList(); |
Signed-off-by: srlch <[email protected]>
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]❌ fail : 143 / 201 (71.14%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I'm doing:
This is part 1 for support backup cluster snapshot:
Introduce SQL Interface for TURN ON and TRUN OFF the automated cluster snapshot task:
Fixes #53867
#53867
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: