Skip to content

Commit

Permalink
Fix two unstable FE UT (#8758)
Browse files Browse the repository at this point in the history 
Use real GlobalStateMgr, not mock GlobalStateMgr, ensure there is only one GlobalStateMgr in UT, then won't Missing invocations issue.
  • Loading branch information
@kangkaisen
kangkaisen committed Jul 18, 2022
1 parent 63e13a4 commit 15babd5
Show file tree
Hide file tree
Showing 18 changed files with 212 additions and 511 deletions.
31 changes: 5 additions & 26 deletions fe/fe-core/src/main/java/com/starrocks/alter/AlterHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,11 @@ protected AlterJob removeAlterJob(long tableId) {
return this.alterJobs.remove(tableId);
}

// For UT
public void clearJobs() {
this.alterJobsV2.clear();
}

@Deprecated
public void removeDbAlterJob(long dbId) {
Iterator<Map.Entry<Long, AlterJob>> iterator = alterJobs.entrySet().iterator();
Expand Down Expand Up @@ -276,32 +281,6 @@ public void handleFinishedReplica(AgentTask task, TTabletInfo finishTabletInfo,
alterJob.handleFinishedReplica(task, finishTabletInfo, reportVersion);
}

/*
* cancel alter job when drop table
* olapTable:
* table which is being dropped
*/
public void cancelWithTable(OlapTable olapTable) {
// make sure to hold to db write lock before calling this
AlterJob alterJob = getAlterJob(olapTable.getId());
if (alterJob == null) {
return;
}
alterJob.cancel(olapTable, "table is dropped");

// remove from alterJobs and add to finishedOrCancelledAlterJobs operation should be perform atomically
lock();
try {
alterJob = alterJobs.remove(olapTable.getId());
if (alterJob != null) {
alterJob.clear();
finishedOrCancelledAlterJobs.add(alterJob);
}
} finally {
unlock();
}
}

protected void cancelInternal(AlterJob alterJob, OlapTable olapTable, String msg) {
// cancel
alterJob.cancel(olapTable, msg);
Expand Down
3 changes: 1 addition & 2 deletions fe/fe-core/src/main/java/com/starrocks/common/util/MasterDaemon.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,11 @@ protected void runOneCycle() {
try {
// not return, but sleep a while. to avoid some thread with large running interval will
// wait for a long time to start again.
Thread.sleep(10 * 1000);
Thread.sleep(100);
} catch (InterruptedException e) {
LOG.warn("interrupted exception. thread: {}", getName(), e);
}
}

runAfterCatalogReady();
}

Expand Down
179 changes: 57 additions & 122 deletions fe/fe-core/src/test/java/com/starrocks/alter/RollupJobV2Test.java
View file

Large diffs are not rendered by default.

118 changes: 29 additions & 89 deletions fe/fe-core/src/test/java/com/starrocks/alter/SchemaChangeJobV2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,17 @@

import com.google.common.collect.Maps;
import com.starrocks.alter.AlterJobV2.JobState;
import com.starrocks.analysis.AccessTestUtil;
import com.starrocks.analysis.AddColumnClause;
import com.starrocks.analysis.AlterClause;
import com.starrocks.analysis.Analyzer;
import com.starrocks.analysis.ColumnDef;
import com.starrocks.analysis.ColumnDef.DefaultValueDef;
import com.starrocks.analysis.ColumnPosition;
import com.starrocks.analysis.ModifyTablePropertiesClause;
import com.starrocks.analysis.StringLiteral;
import com.starrocks.analysis.TypeDef;
import com.starrocks.backup.CatalogMocker;
import com.starrocks.catalog.AggregateType;
import com.starrocks.catalog.Database;
import com.starrocks.catalog.DynamicPartitionProperty;
import com.starrocks.catalog.FakeEditLog;
import com.starrocks.catalog.FakeGlobalStateMgr;
import com.starrocks.catalog.GlobalStateMgrTestUtil;
import com.starrocks.catalog.LocalTablet;
import com.starrocks.catalog.MaterializedIndex;
Expand All @@ -52,22 +47,19 @@
import com.starrocks.catalog.Replica;
import com.starrocks.catalog.ScalarType;
import com.starrocks.catalog.Tablet;
import com.starrocks.common.AnalysisException;
import com.starrocks.common.DdlException;
import com.starrocks.common.FeConstants;
import com.starrocks.common.FeMetaVersion;
import com.starrocks.common.SchemaVersionAndHash;
import com.starrocks.common.UserException;
import com.starrocks.common.jmockit.Deencapsulation;
import com.starrocks.meta.MetaContext;
import com.starrocks.qe.ConnectContext;
import com.starrocks.server.GlobalStateMgr;
import com.starrocks.sql.analyzer.DDLTestBase;
import com.starrocks.task.AgentTask;
import com.starrocks.task.AgentTaskQueue;
import com.starrocks.thrift.TStorageFormat;
import com.starrocks.thrift.TTaskType;
import com.starrocks.transaction.FakeTransactionIDGenerator;
import com.starrocks.transaction.GlobalTransactionMgr;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -80,71 +72,42 @@
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.junit.Assert.assertEquals;

public class SchemaChangeJobV2Test {

public class SchemaChangeJobV2Test extends DDLTestBase {
private static String fileName = "./SchemaChangeV2Test";

private static FakeEditLog fakeEditLog;
private static FakeGlobalStateMgr fakeGlobalStateMgr;
private static FakeTransactionIDGenerator fakeTransactionIDGenerator;
private static GlobalTransactionMgr masterTransMgr;
private static GlobalTransactionMgr slaveTransMgr;
private static GlobalStateMgr masterGlobalStateMgr;
private static GlobalStateMgr slaveGlobalStateMgr;

private static Analyzer analyzer;
private static ColumnDef newCol = new ColumnDef("add_v", new TypeDef(ScalarType.createType(PrimitiveType.INT)),
false, AggregateType.MAX, false, new DefaultValueDef(true, new StringLiteral("1")), "");
private static AddColumnClause addColumnClause = new AddColumnClause(newCol, new ColumnPosition("v"), null, null);
false, null, false, new DefaultValueDef(true, new StringLiteral("1")), "");
private static AddColumnClause addColumnClause = new AddColumnClause(newCol, new ColumnPosition("v3"), null, null);

@Rule
public ExpectedException expectedEx = ExpectedException.none();

@Before
public void setUp() throws InstantiationException, IllegalAccessException, IllegalArgumentException,
InvocationTargetException, NoSuchMethodException, SecurityException, AnalysisException {
fakeEditLog = new FakeEditLog();
fakeGlobalStateMgr = new FakeGlobalStateMgr();
fakeTransactionIDGenerator = new FakeTransactionIDGenerator();
masterGlobalStateMgr = GlobalStateMgrTestUtil.createTestState();
slaveGlobalStateMgr = GlobalStateMgrTestUtil.createTestState();
MetaContext metaContext = new MetaContext();
metaContext.setMetaVersion(FeMetaVersion.VERSION_61);
metaContext.setThreadLocalInfo();

ConnectContext context = new ConnectContext();
context.setStartTime();
context.setThreadLocalInfo();

masterTransMgr = masterGlobalStateMgr.getGlobalTransactionMgr();
masterTransMgr.setEditLog(masterGlobalStateMgr.getEditLog());
slaveTransMgr = slaveGlobalStateMgr.getGlobalTransactionMgr();
slaveTransMgr.setEditLog(slaveGlobalStateMgr.getEditLog());
analyzer = AccessTestUtil.fetchAdminAnalyzer(false);
public void setUp() throws Exception {
super.setUp();
addColumnClause.analyze(analyzer);
}

FeConstants.runningUnitTest = true;
AgentTaskQueue.clearAllTasks();
@After
public void clear() {
GlobalStateMgr.getCurrentState().getSchemaChangeHandler().clearJobs();
}

@Test
public void testAddSchemaChange() throws UserException {
fakeGlobalStateMgr = new FakeGlobalStateMgr();
fakeEditLog = new FakeEditLog();
FakeGlobalStateMgr.setGlobalStateMgr(masterGlobalStateMgr);
SchemaChangeHandler schemaChangeHandler = GlobalStateMgr.getCurrentState().getSchemaChangeHandler();
ArrayList<AlterClause> alterClauses = new ArrayList<>();
alterClauses.add(addColumnClause);
Database db = masterGlobalStateMgr.getDb(GlobalStateMgrTestUtil.testDbId1);
OlapTable olapTable = (OlapTable) db.getTable(GlobalStateMgrTestUtil.testTableId1);

Database db = GlobalStateMgr.getCurrentState().getDb("default_cluster:" + GlobalStateMgrTestUtil.testDb1);
OlapTable olapTable = (OlapTable) db.getTable(GlobalStateMgrTestUtil.testTable1);

schemaChangeHandler.process(alterClauses, db, olapTable);
Map<Long, AlterJobV2> alterJobsV2 = schemaChangeHandler.getAlterJobsV2();
Assert.assertEquals(1, alterJobsV2.size());
Expand All @@ -154,17 +117,16 @@ public void testAddSchemaChange() throws UserException {
// start a schema change, then finished
@Test
public void testSchemaChange1() throws Exception {
fakeGlobalStateMgr = new FakeGlobalStateMgr();
fakeEditLog = new FakeEditLog();
FakeGlobalStateMgr.setGlobalStateMgr(masterGlobalStateMgr);
SchemaChangeHandler schemaChangeHandler = GlobalStateMgr.getCurrentState().getSchemaChangeHandler();

// add a schema change job
ArrayList<AlterClause> alterClauses = new ArrayList<>();
alterClauses.add(addColumnClause);
Database db = masterGlobalStateMgr.getDb(GlobalStateMgrTestUtil.testDbId1);
OlapTable olapTable = (OlapTable) db.getTable(GlobalStateMgrTestUtil.testTableId1);
Partition testPartition = olapTable.getPartition(GlobalStateMgrTestUtil.testPartitionId1);

Database db = GlobalStateMgr.getCurrentState().getDb("default_cluster:" + GlobalStateMgrTestUtil.testDb1);
OlapTable olapTable = (OlapTable) db.getTable(GlobalStateMgrTestUtil.testTable1);
Partition testPartition = olapTable.getPartition(GlobalStateMgrTestUtil.testTable1);

schemaChangeHandler.process(alterClauses, db, olapTable);
Map<Long, AlterJobV2> alterJobsV2 = schemaChangeHandler.getAlterJobsV2();
Assert.assertEquals(1, alterJobsV2.size());
Expand All @@ -178,18 +140,10 @@ public void testSchemaChange1() throws Exception {
LocalTablet baseTablet = (LocalTablet) baseIndex.getTablets().get(0);
List<Replica> replicas = baseTablet.getReplicas();
Replica replica1 = replicas.get(0);
Replica replica2 = replicas.get(1);
Replica replica3 = replicas.get(2);

assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica1.getVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica2.getVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica3.getVersion());
assertEquals(1, replica1.getVersion());
assertEquals(-1, replica1.getLastFailedVersion());
assertEquals(-1, replica2.getLastFailedVersion());
assertEquals(-1, replica3.getLastFailedVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica1.getLastSuccessVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica2.getLastSuccessVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica3.getLastSuccessVersion());
assertEquals(1, replica1.getLastSuccessVersion());

// runPendingJob
schemaChangeHandler.runAfterCatalogReady();
Expand Down Expand Up @@ -232,17 +186,16 @@ public void testSchemaChange1() throws Exception {

@Test
public void testSchemaChangeWhileTabletNotStable() throws Exception {
fakeGlobalStateMgr = new FakeGlobalStateMgr();
fakeEditLog = new FakeEditLog();
FakeGlobalStateMgr.setGlobalStateMgr(masterGlobalStateMgr);
SchemaChangeHandler schemaChangeHandler = GlobalStateMgr.getCurrentState().getSchemaChangeHandler();

// add a schema change job
ArrayList<AlterClause> alterClauses = new ArrayList<>();
alterClauses.add(addColumnClause);
Database db = masterGlobalStateMgr.getDb(GlobalStateMgrTestUtil.testDbId1);
OlapTable olapTable = (OlapTable) db.getTable(GlobalStateMgrTestUtil.testTableId1);
Partition testPartition = olapTable.getPartition(GlobalStateMgrTestUtil.testPartitionId1);

Database db = GlobalStateMgr.getCurrentState().getDb("default_cluster:" + GlobalStateMgrTestUtil.testDb1);
OlapTable olapTable = (OlapTable) db.getTable(GlobalStateMgrTestUtil.testTable1);
Partition testPartition = olapTable.getPartition(GlobalStateMgrTestUtil.testTable1);

schemaChangeHandler.process(alterClauses, db, olapTable);
Map<Long, AlterJobV2> alterJobsV2 = schemaChangeHandler.getAlterJobsV2();
Assert.assertEquals(1, alterJobsV2.size());
Expand All @@ -256,18 +209,10 @@ public void testSchemaChangeWhileTabletNotStable() throws Exception {
LocalTablet baseTablet = (LocalTablet) baseIndex.getTablets().get(0);
List<Replica> replicas = baseTablet.getReplicas();
Replica replica1 = replicas.get(0);
Replica replica2 = replicas.get(1);
Replica replica3 = replicas.get(2);

assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica1.getVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica2.getVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica3.getVersion());
assertEquals(1, replica1.getVersion());
assertEquals(-1, replica1.getLastFailedVersion());
assertEquals(-1, replica2.getLastFailedVersion());
assertEquals(-1, replica3.getLastFailedVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica1.getLastSuccessVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica2.getLastSuccessVersion());
assertEquals(GlobalStateMgrTestUtil.testStartVersion, replica3.getLastSuccessVersion());
assertEquals(1, replica1.getLastSuccessVersion());

// runPendingJob
replica1.setState(Replica.ReplicaState.DECOMMISSION);
Expand Down Expand Up @@ -316,9 +261,6 @@ public void testSchemaChangeWhileTabletNotStable() throws Exception {

@Test
public void testModifyDynamicPartitionNormal() throws UserException {
fakeGlobalStateMgr = new FakeGlobalStateMgr();
fakeEditLog = new FakeEditLog();
FakeGlobalStateMgr.setGlobalStateMgr(masterGlobalStateMgr);
SchemaChangeHandler schemaChangeHandler = GlobalStateMgr.getCurrentState().getSchemaChangeHandler();
ArrayList<AlterClause> alterClauses = new ArrayList<>();
Map<String, String> properties = new HashMap<>();
Expand Down Expand Up @@ -373,8 +315,6 @@ public void testModifyDynamicPartitionNormal() throws UserException {
public void modifyDynamicPartitionWithoutTableProperty(String propertyKey, String propertyValue,
String missPropertyKey)
throws UserException {
fakeGlobalStateMgr = new FakeGlobalStateMgr();
FakeGlobalStateMgr.setGlobalStateMgr(masterGlobalStateMgr);
SchemaChangeHandler schemaChangeHandler = GlobalStateMgr.getCurrentState().getSchemaChangeHandler();
ArrayList<AlterClause> alterClauses = new ArrayList<>();
Map<String, String> properties = new HashMap<>();
Expand Down

0 comments on commit 15babd5

Please sign in to comment.