[Feature] Support Cluster Snapshot Backup: checkpoint and image backup (part3) #54695

srlch · 2025-01-04T08:45:41Z

What I'm doing:

This is part3 for supporting cluster snapshot backport for share data mode.

In this pr, we introduce a new checkpoint controller to finish the image backup:

Introduce ClusterSnapshotCheckpointController to control the creation of the imgae
for FE and StarMgr in the same thread with consistent journal ids between two image.
ClusterSnapshotCheckpointController should be exclusive with the normal checkpoint.
This controller will responsible for create image in leader node and upload it into remote storage.

Fixes #53867
#53867

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

xiangguangyxg · 2025-01-06T07:02:27Z

fe/fe-core/src/main/java/com/starrocks/fs/hdfs/HdfsFsManager.java

+            File srcFile = new File(srcPath);
+            FileUtil.copy(srcFile, fileSystem.getDFSFileSystem(), new Path(destPathUri.getPath()), false, new Configuration());


could use fileSystem.copyFromLocalFile() directly
https://hadoop.apache.org/docs/r3.4.0/api/org/apache/hadoop/fs/FileSystem.html#copyFromLocalFile-boolean-boolean-org.apache.hadoop.fs.Path-org.apache.hadoop.fs.Path-

It seems that copyFromLocalFile will failed if the src is not a file

xiangguangyxg · 2025-01-06T07:09:13Z

fe/fe-core/src/main/java/com/starrocks/fs/hdfs/HdfsFsManager.java

+            LOG.error("Interrupted while copy from local {} to {} ", srcPath, destPath + " " + e.getMessage());
+            throw new StarRocksException("Failed to copy " + destPath + " from local " + srcPath + " " + e.getMessage());
+        } catch (Exception e) {
+            LOG.error("Exception while copy from local {} to {} ", srcPath, destPath + " " + e.getMessage());
+            throw new StarRocksException("Failed to copy " + destPath + " from local " + srcPath  + " " + e.getMessage());


Suggested change

LOG.error("Interrupted while copy from local {} to {} ", srcPath, destPath + " " + e.getMessage());

throw new StarRocksException("Failed to copy " + destPath + " from local " + srcPath + " " + e.getMessage());

} catch (Exception e) {

LOG.error("Exception while copy from local {} to {} ", srcPath, destPath + " " + e.getMessage());

throw new StarRocksException("Failed to copy " + destPath + " from local " + srcPath + " " + e.getMessage());

LOG.error("Interrupted while copy local {} to {} ", srcPath, destPath, e);

throw new StarRocksException("Failed to copy local " + srcPath + " to " + destPath, e);

} catch (Exception e) {

LOG.error("Exception while copy local {} to {} ", srcPath, destPath, e);

throw new StarRocksException("Failed to copy local " + srcPath + " to " + destPath, e);

xiangguangyxg · 2025-01-06T07:10:06Z

fe/fe-core/src/main/java/com/starrocks/fs/hdfs/HdfsService.java

@@ -59,6 +59,11 @@ public void copyToLocal(String srcPath, String destPath, Map<String, String> pro
        LOG.info("Copied {} to local {}", srcPath, destPath);
    }

+    public void copyFromLocal(String srcPath, String destPath, Map<String, String> properties) throws StarRocksException {
+        fileSystemManager.copyFromLocal(srcPath, destPath, properties);
+        LOG.info("Copied {} from local {}", destPath, srcPath);


Suggested change

LOG.info("Copied {} from local {}", destPath, srcPath);

LOG.info("Copied local {} to {}", srcPath, destPath);

xiangguangyxg · 2025-01-06T07:10:43Z

fe/fe-core/src/main/java/com/starrocks/journal/CheckpointWorker.java

            CheckpointController controller = getCheckpointController();
+            if (CheckpointController.clusterSnapshotCheckpointRunning()) {
+                controller = GlobalStateMgr.getServingState().getClusterSnapshotCheckpointController();
+            } else {
+                controller = getCheckpointController();
+            }


line 137 and 141 are duplicated ?

xiangguangyxg · 2025-01-06T07:13:40Z

fe/fe-core/src/main/java/com/starrocks/lake/snapshot/ClusterSnapshot.java

+    @SerializedName(value = "createTime")
+    private long createTime;
+    @SerializedName(value = "successTime")
+    private long successTime;


make the veriable name same to the corresponding colume name in system table ?

xiangguangyxg · 2025-01-06T07:14:17Z

fe/fe-core/src/main/java/com/starrocks/lake/snapshot/ClusterSnapshotJob.java

+    @SerializedName(value = "createTime")
+    private long createTime;
+    @SerializedName(value = "successTime")
+    private long successTime;


same to the above

sonarqubecloud · 2025-01-06T09:49:49Z

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

gengjun-git · 2025-01-07T06:17:14Z

fe/fe-core/src/main/java/com/starrocks/leader/CheckpointController.java

+        changeRunningState(false);
+    }
+
+    private boolean selfIsRunning() {


How about put the waiting code into subClass?

BaseController / \ / \ NormalController SnapshotController

The logic will be more clear.
Self and peer is a little obscure. And it is not reasonable to let the base class perceive the subclass

updated, refactor code and introduce exclusive flag for checkpoint controller in base class

…p (part3) Signed-off-by: srlch <[email protected]>

github-actions · 2025-01-07T08:20:10Z

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2025-01-07T08:20:12Z

[FE Incremental Coverage Report]

❌ fail : 16 / 38 (42.11%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	com/starrocks/lake/StarOSAgent.java	0	3	00.00%	[143, 144, 145]
🔵	com/starrocks/fs/hdfs/HdfsFsManager.java	0	12	00.00%	[1217, 1218, 1219, 1220, 1221, 1222, 1223, 1227, 1228, 1229, 1231, 1232]
🔵	com/starrocks/fs/hdfs/HdfsService.java	0	4	00.00%	[62, 63, 64, 66]
🔵	com/starrocks/fs/HdfsUtil.java	3	4	75.00%	[78]
🔵	com/starrocks/server/GlobalStateMgr.java	8	10	80.00%	[2231, 2232]
🔵	com/starrocks/server/NodeMgr.java	1	1	100.00%	[]
🔵	com/starrocks/storagevolume/StorageVolume.java	4	4	100.00%	[]

github-actions · 2025-01-07T08:22:01Z

[BE Incremental Coverage Report]

❌ fail : 0 / 14 (00.00%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	be/src/exec/schema_scanner/schema_cluster_snapshots_scanner.cpp	0	8	00.00%	[57, 58, 60, 62, 64, 65, 66, 67]
🔵	be/src/exec/schema_scanner/schema_cluster_snapshot_jobs_scanner.cpp	0	6	00.00%	[55, 56, 58, 60, 62, 63]

srlch requested review from a team as code owners January 4, 2025 08:45

wanpengfei-git added the META-REVIEW label Jan 4, 2025

github-actions bot added the title needs [type] label Jan 4, 2025

wanpengfei-git requested a review from a team January 4, 2025 08:46

mergify bot assigned srlch Jan 4, 2025

srlch changed the title ~~[WIP] Support Cluster Snapshot Backup: checkpoint and image backup (part3)~~ [Feature] Support Cluster Snapshot Backup: checkpoint and image backup (part3) Jan 5, 2025

srlch force-pushed the cluster_snapshot_part_3_new branch from 4d88ab3 to d723b6f Compare January 5, 2025 08:41

srlch requested a review from a team as a code owner January 5, 2025 08:41

github-actions bot removed the title needs [type] label Jan 5, 2025

srlch force-pushed the cluster_snapshot_part_3_new branch 2 times, most recently from 7ea70c7 to 5547bbd Compare January 6, 2025 06:30

gengjun-git self-assigned this Jan 6, 2025

xiangguangyxg reviewed Jan 6, 2025

View reviewed changes

gengjun-git reviewed Jan 7, 2025

View reviewed changes

[Feature] Support Cluster Snapshot Backup: checkpoint and image backu…

049527d

…p (part3) Signed-off-by: srlch <[email protected]>

srlch force-pushed the cluster_snapshot_part_3_new branch from 9d7db2f to 049527d Compare January 7, 2025 08:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Support Cluster Snapshot Backup: checkpoint and image backup (part3) #54695

[Feature] Support Cluster Snapshot Backup: checkpoint and image backup (part3) #54695

srlch commented Jan 4, 2025 •

edited

Loading

xiangguangyxg Jan 6, 2025

srlch Jan 6, 2025

xiangguangyxg Jan 6, 2025

srlch Jan 6, 2025

xiangguangyxg Jan 6, 2025

srlch Jan 6, 2025

xiangguangyxg Jan 6, 2025

srlch Jan 6, 2025

xiangguangyxg Jan 6, 2025

srlch Jan 6, 2025

xiangguangyxg Jan 6, 2025

srlch Jan 6, 2025

sonarqubecloud bot commented Jan 6, 2025

gengjun-git Jan 7, 2025 •

edited

Loading

srlch Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

		File srcFile = new File(srcPath);
		FileUtil.copy(srcFile, fileSystem.getDFSFileSystem(), new Path(destPathUri.getPath()), false, new Configuration());

	LOG.info("Copied {} from local {}", destPath, srcPath);
	LOG.info("Copied local {} to {}", srcPath, destPath);

[Feature] Support Cluster Snapshot Backup: checkpoint and image backup (part3) #54695

Are you sure you want to change the base?

[Feature] Support Cluster Snapshot Backup: checkpoint and image backup (part3) #54695

Conversation

srlch commented Jan 4, 2025 • edited Loading

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 6, 2025

Quality Gate failed

gengjun-git Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 7, 2025

[Java-Extensions Incremental Coverage Report]

github-actions bot commented Jan 7, 2025

[FE Incremental Coverage Report]

file detail

github-actions bot commented Jan 7, 2025

[BE Incremental Coverage Report]

file detail

srlch commented Jan 4, 2025 •

edited

Loading

gengjun-git Jan 7, 2025 •

edited

Loading