w41ter commented on code in PR #47314:
URL: https://github.com/apache/doris/pull/47314#discussion_r1928048046
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -215,6 +216,8 @@ public enum RestoreJobState {
private boolean isCleanPartitions = false;
// Whether to restore the data into a temp table, and then replace the
origin one.
private boolean isAtomicRestore = false;
+ // Whether to restore the table replace with the exists table.
Review Comment:
```suggestion
// Whether to restore the table by replacing the exists but conflicted
table.
```
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -252,23 +256,26 @@ public RestoreJob(String label, String backupTs, long
dbId, String dbName, Backu
this.isCleanTables = isCleanTables;
this.isCleanPartitions = isCleanPartitions;
this.isAtomicRestore = isAtomicRestore;
+ this.isForceReplace = isForceReplace;
Review Comment:
```suggestion
if (this.isAtomicRestore) {
this.isForceReplace = isForceReplace;
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -1294,6 +1330,9 @@ private boolean
genFileMappingWhenBackupReplicasEqual(PartitionInfo localPartInf
restoreReplicaNum = remoteReplicaAlloc.getTotalReplicaNum();
}
if (localReplicaNum != restoreReplicaNum) {
+ if (isForceReplace) {
+ return true;
+ }
Review Comment:
```suggestion
```
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -2060,8 +2099,8 @@ private Status allTabletCommitted(boolean isReplay) {
}
// replace the origin tables in atomic.
- if (isAtomicRestore) {
- Status st = atomicReplaceOlapTables(db, isReplay);
+ if (isAtomicRestore || isForceReplace) {
+ Status st = restoreWithReplaceOlapTables(db, isReplay);
Review Comment:
```suggestion
if (isAtomicRestore) {
Status st = atomicReplaceOlapTables(db, isReplay);
```
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -691,31 +698,44 @@ private void checkAndPrepareMeta() {
Table remoteTbl = backupMeta.getTable(tableName);
Preconditions.checkNotNull(remoteTbl);
Table localTbl =
db.getTableNullable(jobInfo.getAliasByOriginNameIfSet(tableName));
+ boolean isSchemaChanged = false;
if (localTbl != null && localTbl.getType() != TableType.OLAP) {
- // table already exist, but is not OLAP
- status = new Status(ErrCode.COMMON_ERROR,
- "The type of local table should be same as type of
remote table: "
- + remoteTbl.getName());
- return;
+ if (!isForceReplace) {
+ isSchemaChanged = true;
+ } else {
+ // table already exist, but is not OLAP
+ status = new Status(ErrCode.COMMON_ERROR,
+ "The type of local table should be same as
type of remote table: "
+ + remoteTbl.getName());
+ return;
+ }
}
if (localTbl != null) {
OlapTable localOlapTbl = (OlapTable) localTbl;
OlapTable remoteOlapTbl = (OlapTable) remoteTbl;
if (localOlapTbl.isColocateTable() || (reserveColocate &&
remoteOlapTbl.isColocateTable())) {
- status = new Status(ErrCode.COMMON_ERROR, "Not support
to restore to local table "
- + tableName + " with colocate group.");
- return;
+ if (!isForceReplace) {
+ isSchemaChanged = true;
+ } else {
+ status = new Status(ErrCode.COMMON_ERROR, "Not
support to restore to local table "
+ + tableName + " with colocate group.");
+ return;
+ }
Review Comment:
Keep the original logic for now, as we don't support the colocate group yet.
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -691,31 +698,44 @@ private void checkAndPrepareMeta() {
Table remoteTbl = backupMeta.getTable(tableName);
Preconditions.checkNotNull(remoteTbl);
Table localTbl =
db.getTableNullable(jobInfo.getAliasByOriginNameIfSet(tableName));
+ boolean isSchemaChanged = false;
if (localTbl != null && localTbl.getType() != TableType.OLAP) {
- // table already exist, but is not OLAP
- status = new Status(ErrCode.COMMON_ERROR,
- "The type of local table should be same as type of
remote table: "
- + remoteTbl.getName());
- return;
+ if (!isForceReplace) {
+ isSchemaChanged = true;
Review Comment:
Consider adding some logs here
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -725,13 +745,17 @@ private void checkAndPrepareMeta() {
String remoteTblSignature = remoteOlapTbl.getSignature(
BackupHandler.SIGNATURE_VERSION,
intersectPartNames);
if (!localTblSignature.equals(remoteTblSignature)) {
- String alias =
jobInfo.getAliasByOriginNameIfSet(tableName);
- LOG.warn("Table {} already exists but with
different schema, "
- + "local table: {}, remote table: {}",
- alias, localTblSignature,
remoteTblSignature);
- status = new Status(ErrCode.COMMON_ERROR, "Table "
- + alias + " already exist but with
different schema");
- return;
+ if (isForceReplace) {
+ isSchemaChanged = true;
Review Comment:
Consider adding some logs here
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -768,7 +796,11 @@ private void checkAndPrepareMeta() {
// Same partition, same range or a single
partitioned table.
if
(genFileMappingWhenBackupReplicasEqual(localPartInfo, localPartition,
localTbl, backupPartInfo,
partitionName, tblInfo, remoteReplicaAlloc)) {
- return;
+ if (isForceReplace) {
+ isSchemaChanged = true;
+ } else {
+ return;
+ }
Review Comment:
```suggestion
return;
```
Atomic restore will skip this step.
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -691,31 +698,44 @@ private void checkAndPrepareMeta() {
Table remoteTbl = backupMeta.getTable(tableName);
Preconditions.checkNotNull(remoteTbl);
Table localTbl =
db.getTableNullable(jobInfo.getAliasByOriginNameIfSet(tableName));
+ boolean isSchemaChanged = false;
if (localTbl != null && localTbl.getType() != TableType.OLAP) {
- // table already exist, but is not OLAP
- status = new Status(ErrCode.COMMON_ERROR,
- "The type of local table should be same as type of
remote table: "
- + remoteTbl.getName());
- return;
+ if (!isForceReplace) {
+ isSchemaChanged = true;
+ } else {
+ // table already exist, but is not OLAP
+ status = new Status(ErrCode.COMMON_ERROR,
+ "The type of local table should be same as
type of remote table: "
+ + remoteTbl.getName());
+ return;
+ }
}
if (localTbl != null) {
OlapTable localOlapTbl = (OlapTable) localTbl;
OlapTable remoteOlapTbl = (OlapTable) remoteTbl;
if (localOlapTbl.isColocateTable() || (reserveColocate &&
remoteOlapTbl.isColocateTable())) {
- status = new Status(ErrCode.COMMON_ERROR, "Not support
to restore to local table "
- + tableName + " with colocate group.");
- return;
+ if (!isForceReplace) {
+ isSchemaChanged = true;
+ } else {
+ status = new Status(ErrCode.COMMON_ERROR, "Not
support to restore to local table "
+ + tableName + " with colocate group.");
+ return;
+ }
}
localOlapTbl.readLock();
try {
List<String> intersectPartNames = Lists.newArrayList();
Status st =
localOlapTbl.getIntersectPartNamesWith(remoteOlapTbl, intersectPartNames);
if (!st.ok()) {
- status = st;
- return;
+ if (isForceReplace) {
+ isSchemaChanged = true;
Review Comment:
Consider adding some logs here
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -778,10 +810,14 @@ private void checkAndPrepareMeta() {
PartitionItem remoteItem =
remoteOlapTbl.getPartitionInfo()
.getItem(backupPartInfo.id);
if
(localPartitionInfo.getAnyIntersectItem(remoteItem, false) != null) {
- status = new
Status(ErrCode.COMMON_ERROR, "Partition " + partitionName
- + " in table " +
localTbl.getName()
- + " has conflict partition
item with existing items");
- return;
+ if (isForceReplace) {
+ isSchemaChanged = true;
+ } else {
+ status = new
Status(ErrCode.COMMON_ERROR, "Partition " + partitionName
+ + " in table " +
localTbl.getName()
+ + " has conflict partition
item with existing items");
+ return;
+ }
Review Comment:
Keep the original logic, as the atomic restore won't step in here.
##########
fe/fe-core/src/main/java/org/apache/doris/backup/RestoreJob.java:
##########
@@ -2427,8 +2466,7 @@ private void cancelInternal(boolean isReplay) {
LOG.info("finished to cancel restore job. is replay: {}. {}",
isReplay, this);
}
- private Status atomicReplaceOlapTables(Database db, boolean isReplay) {
- assert isAtomicRestore;
+ private Status restoreWithReplaceOlapTables(Database db, boolean isReplay)
{
Review Comment:
The name atomicReplaceOlapTables is enough, please revert the below changes.
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
