You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by ya...@apache.org on 2021/04/19 01:26:31 UTC
[incubator-doris] branch master updated: [Bug] Fix bug that tablets
are not dropped when replacing tables (#5627)
This is an automated email from the ASF dual-hosted git repository.
yangzhg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-doris.git
The following commit(s) were added to refs/heads/master by this push:
new 6be03f3 [Bug] Fix bug that tablets are not dropped when replacing tables (#5627)
6be03f3 is described below
commit 6be03f339cec5fdeeee185557ab7f6d612f416a8
Author: Mingyu Chen <mo...@gmail.com>
AuthorDate: Mon Apr 19 09:26:19 2021 +0800
[Bug] Fix bug that tablets are not dropped when replacing tables (#5627)
When replacing table with swap = false, the origin table's tablets
should be removed from tablet inverted index.
Co-authored-by: xxiao2018 <be...@sina.com>
---
.../main/java/org/apache/doris/alter/Alter.java | 10 ++-
.../java/org/apache/doris/alter/AlterTest.java | 84 +++++++++++++++++++++-
2 files changed, 89 insertions(+), 5 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
index 0219d4b..694126a 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
@@ -295,7 +295,7 @@ public class Alter {
if (swapTable) {
origTable.checkAndSetName(newTblName, true);
}
- replaceTableInternal(db, origTable, olapNewTbl, swapTable);
+ replaceTableInternal(db, origTable, olapNewTbl, swapTable, false);
// write edit log
ReplaceTableOperationLog log = new ReplaceTableOperationLog(db.getId(), origTable.getId(), olapNewTbl.getId(), swapTable);
Catalog.getCurrentCatalog().getEditLog().logReplaceTable(log);
@@ -316,7 +316,7 @@ public class Alter {
OlapTable newTbl = (OlapTable) db.getTable(newTblId);
try {
- replaceTableInternal(db, origTable, newTbl, log.isSwapTable());
+ replaceTableInternal(db, origTable, newTbl, log.isSwapTable(), true);
} catch (DdlException e) {
LOG.warn("should not happen", e);
}
@@ -337,7 +337,8 @@ public class Alter {
* 1.1 check if B can be renamed to A (checking name conflict, etc...)
* 1.2 rename B to A, drop old A, and add new A to database.
*/
- private void replaceTableInternal(Database db, OlapTable origTable, OlapTable newTbl, boolean swapTable)
+ private void replaceTableInternal(Database db, OlapTable origTable, OlapTable newTbl, boolean swapTable,
+ boolean isReplay)
throws DdlException {
String oldTblName = origTable.getName();
String newTblName = newTbl.getName();
@@ -354,6 +355,9 @@ public class Alter {
// rename origin table name to new table name and add it to database
origTable.checkAndSetName(newTblName, false);
db.createTable(origTable);
+ } else {
+ // not swap, the origin table is not used anymore, need to drop all its tablets.
+ Catalog.getCurrentCatalog().onEraseOlapTable(origTable, isReplay);
}
}
diff --git a/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java b/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java
index f371f02..6aa7148 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/alter/AlterTest.java
@@ -29,6 +29,8 @@ import org.apache.doris.catalog.OlapTable;
import org.apache.doris.catalog.Partition;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.Table;
+import org.apache.doris.catalog.Tablet;
+import org.apache.doris.catalog.TabletInvertedIndex;
import org.apache.doris.catalog.Type;
import org.apache.doris.common.Config;
import org.apache.doris.common.FeConstants;
@@ -37,13 +39,14 @@ import org.apache.doris.qe.ConnectContext;
import org.apache.doris.thrift.TStorageMedium;
import org.apache.doris.utframe.UtFrameUtils;
-import com.google.common.collect.Lists;
-
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
import java.io.File;
import java.util.List;
import java.util.Map;
@@ -531,6 +534,49 @@ public class AlterTest {
createTable(stmt4);
Database db = Catalog.getCurrentCatalog().getDb("default_cluster:test");
+ // table name -> tabletIds
+ Map<String, List<Long>> tblNameToTabletIds = Maps.newHashMap();
+ OlapTable replace1Tbl = (OlapTable) db.getTable("replace1");
+ OlapTable r1Tbl = (OlapTable) db.getTable("r1");
+ OlapTable replace2Tbl = (OlapTable) db.getTable("replace2");
+ OlapTable replace3Tbl = (OlapTable) db.getTable("replace3");
+
+ tblNameToTabletIds.put("replace1", Lists.newArrayList());
+ for (Partition partition : replace1Tbl.getAllPartitions()) {
+ for (MaterializedIndex index : partition.getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE)) {
+ for (Tablet tablet : index.getTablets()) {
+ tblNameToTabletIds.get("replace1").add(tablet.getId());
+ }
+ }
+ }
+
+ tblNameToTabletIds.put("r1", Lists.newArrayList());
+ for (Partition partition : r1Tbl.getAllPartitions()) {
+ for (MaterializedIndex index : partition.getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE)) {
+ for (Tablet tablet : index.getTablets()) {
+ tblNameToTabletIds.get("r1").add(tablet.getId());
+ }
+ }
+ }
+
+ tblNameToTabletIds.put("replace2", Lists.newArrayList());
+ for (Partition partition : replace2Tbl.getAllPartitions()) {
+ for (MaterializedIndex index : partition.getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE)) {
+ for (Tablet tablet : index.getTablets()) {
+ tblNameToTabletIds.get("replace2").add(tablet.getId());
+ }
+ }
+ }
+
+ tblNameToTabletIds.put("replace3", Lists.newArrayList());
+ for (Partition partition : replace3Tbl.getAllPartitions()) {
+ for (MaterializedIndex index : partition.getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE)) {
+ for (Tablet tablet : index.getTablets()) {
+ tblNameToTabletIds.get("replace3").add(tablet.getId());
+ }
+ }
+ }
+
// name conflict
String replaceStmt = "ALTER TABLE test.replace1 REPLACE WITH TABLE r1";
alterTable(replaceStmt, true);
@@ -543,6 +589,8 @@ public class AlterTest {
Assert.assertEquals(1, replace2.getPartition("replace2").getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE).size());
alterTable(replaceStmt, false);
+ Assert.assertTrue(checkAllTabletsExists(tblNameToTabletIds.get("replace1")));
+ Assert.assertTrue(checkAllTabletsExists(tblNameToTabletIds.get("replace2")));
replace1 = (OlapTable) db.getTable("replace1");
replace2 = (OlapTable) db.getTable("replace2");
@@ -559,6 +607,8 @@ public class AlterTest {
Assert.assertNull(replace2);
Assert.assertEquals(3, replace1.getPartition("replace1").getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE).size());
Assert.assertEquals("replace1", replace1.getIndexNameById(replace1.getBaseIndexId()));
+ Assert.assertTrue(checkAllTabletsNotExists(tblNameToTabletIds.get("replace2")));
+ Assert.assertTrue(checkAllTabletsExists(tblNameToTabletIds.get("replace1")));
replaceStmt = "ALTER TABLE test.replace1 REPLACE WITH TABLE replace3 properties('swap' = 'true')";
alterTable(replaceStmt, false);
@@ -569,11 +619,41 @@ public class AlterTest {
Assert.assertNotNull(replace1.getIndexIdByName("r3"));
Assert.assertNotNull(replace1.getIndexIdByName("r4"));
+ Assert.assertTrue(checkAllTabletsExists(tblNameToTabletIds.get("replace1")));
+ Assert.assertTrue(checkAllTabletsExists(tblNameToTabletIds.get("replace3")));
+
Assert.assertEquals(3, replace3.getPartition("replace3").getMaterializedIndices(MaterializedIndex.IndexExtState.VISIBLE).size());
Assert.assertNotNull(replace3.getIndexIdByName("r1"));
Assert.assertNotNull(replace3.getIndexIdByName("r2"));
}
+ private boolean checkAllTabletsExists(List<Long> tabletIds) {
+ TabletInvertedIndex invertedIndex = Catalog.getCurrentCatalog().getTabletInvertedIndex();
+ for (long tabletId : tabletIds) {
+ if (invertedIndex.getTabletMeta(tabletId) == null) {
+ return false;
+ }
+ if (invertedIndex.getReplicasByTabletId(tabletId).isEmpty()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private boolean checkAllTabletsNotExists(List<Long> tabletIds) {
+ TabletInvertedIndex invertedIndex = Catalog.getCurrentCatalog().getTabletInvertedIndex();
+ for (long tabletId : tabletIds) {
+ if (invertedIndex.getTabletMeta(tabletId) != null) {
+ return false;
+ }
+
+ if (!invertedIndex.getReplicasByTabletId(tabletId).isEmpty()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
@Test
public void testExternalTableAlterOperations() throws Exception {
// external table do not support partition operation
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org