Skip to content

Commit c07486e

Browse files
dengzhhu653dengzhhu653
committed
review-2
1 parent a61dab7 commit c07486e

File tree

6 files changed

+67
-109
lines changed

6 files changed

+67
-109
lines changed

‎itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/MetastoreHousekeepingLeaderTestBase.java‎

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public abstract class MetastoreHousekeepingLeaderTestBase {
5656
static Map<String, Boolean> threadNames = new HashMap<>();
5757
static Map<Class<? extends Thread>, Boolean> threadClasses = new HashMap<>();
5858

59-
void internalSetup(final String leaderHostName, Configuration configuration) throws Exception {
59+
void setup(final String leaderHostName, Configuration configuration) throws Exception {
6060
this.conf = configuration;
6161
MetaStoreTestUtils.setConfForStandloneMode(conf);
6262
MetastoreConf.setVar(conf, ConfVars.THRIFT_BIND_HOST, "localhost");
@@ -223,9 +223,52 @@ public void takeLeadership(LeaderElection election) throws Exception {
223223
@Override
224224
public void lossLeadership(LeaderElection election) throws Exception {
225225
if (latch != null) {
226+
// This is the last one get notified, sleep some time to make sure all other
227+
// services have been stopped before return
228+
Thread.sleep(12000);
226229
latch.countDown();
227230
}
228231
}
229232
}
233+
234+
public void checkHouseKeepingThreadExistence(boolean isLeader) throws Exception {
235+
searchHousekeepingThreads();
236+
237+
// Verify existence of threads
238+
for (Map.Entry<String, Boolean> entry : threadNames.entrySet()) {
239+
if (entry.getValue()) {
240+
LOG.info("Found thread with name {}", entry.getKey());
241+
} else {
242+
LOG.info("No thread found with name {}", entry.getKey());
243+
}
244+
if (isLeader) {
245+
Assert.assertTrue("No thread with name " + entry.getKey() + " found.", entry.getValue());
246+
} else {
247+
Assert.assertFalse("Thread with name " + entry.getKey() + " found.", entry.getValue());
248+
}
249+
}
250+
251+
for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
252+
if (isLeader) {
253+
if (entry.getValue()) {
254+
LOG.info("Found thread for {}", entry.getKey().getSimpleName());
255+
}
256+
Assert.assertTrue("No thread found for class " + entry.getKey().getSimpleName(), entry.getValue());
257+
} else {
258+
// A non-leader HMS will still run the configured number of Compaction worker threads.
259+
if (entry.getKey() == Worker.class) {
260+
if (entry.getValue()) {
261+
LOG.info("Thread found for " + entry.getKey().getSimpleName());
262+
}
263+
} else {
264+
if (!entry.getValue()) {
265+
LOG.info("No thread found for " + entry.getKey().getSimpleName());
266+
}
267+
Assert.assertFalse("Thread found for class " + entry.getKey().getSimpleName(),
268+
entry.getValue());
269+
}
270+
}
271+
}
272+
}
230273
}
231274

‎itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeader.java‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,44 +19,23 @@
1919
package org.apache.hadoop.hive.metastore;
2020

2121
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
22-
import org.junit.Assert;
2322
import org.junit.Before;
2423
import org.junit.Test;
25-
import org.slf4j.Logger;
26-
import org.slf4j.LoggerFactory;
27-
28-
import java.util.Map;
2924

3025
/**
3126
* Test for specifying a valid hostname as HMS leader.
3227
*/
3328
public class TestMetastoreHousekeepingLeader extends MetastoreHousekeepingLeaderTestBase {
34-
private static final Logger LOG = LoggerFactory.getLogger(TestMetastoreHousekeepingLeader.class);
3529

3630
@Before
3731
public void setUp() throws Exception {
38-
internalSetup("localhost", MetastoreConf.newMetastoreConf());
32+
setup("localhost", MetastoreConf.newMetastoreConf());
3933
}
4034

4135
@Test
4236
public void testHouseKeepingThreadExistence() throws Exception {
43-
searchHousekeepingThreads();
44-
45-
// Verify existence of threads
46-
for (Map.Entry<String, Boolean> entry : threadNames.entrySet()) {
47-
if (entry.getValue()) {
48-
LOG.info("Found thread with name " + entry.getKey());
49-
}
50-
Assert.assertTrue("No thread with name " + entry.getKey() + " found.", entry.getValue());
51-
}
52-
53-
for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
54-
if (entry.getValue()) {
55-
LOG.info("Found thread for " + entry.getKey().getSimpleName());
56-
}
57-
Assert.assertTrue("No thread found for class " + entry.getKey().getSimpleName(),
58-
entry.getValue());
59-
}
37+
checkHouseKeepingThreadExistence(true);
6038
}
39+
6140
}
6241

‎itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingLeaderEmptyConfig.java‎

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,45 +19,23 @@
1919
package org.apache.hadoop.hive.metastore;
2020

2121
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
22-
import org.junit.Assert;
2322
import org.junit.Before;
2423
import org.junit.Test;
25-
import org.slf4j.Logger;
26-
import org.slf4j.LoggerFactory;
27-
28-
import java.util.Map;
2924

3025
/**
3126
* Test for specifying empty HMS leader.
3227
*/
3328
public class TestMetastoreHousekeepingLeaderEmptyConfig extends MetastoreHousekeepingLeaderTestBase {
34-
private static final Logger LOG =
35-
LoggerFactory.getLogger(TestMetastoreHousekeepingLeaderEmptyConfig.class);
3629

3730
@Before
3831
public void setUp() throws Exception {
3932
// Empty string for leader indicates that the HMS is leader.
40-
internalSetup("", MetastoreConf.newMetastoreConf());
33+
setup("", MetastoreConf.newMetastoreConf());
4134
}
4235

4336
@Test
4437
public void testHouseKeepingThreadExistence() throws Exception {
45-
searchHousekeepingThreads();
46-
47-
// Verify existence of threads
48-
for (Map.Entry<String, Boolean> entry : threadNames.entrySet()) {
49-
if (entry.getValue()) {
50-
LOG.info("Found thread with name " + entry.getKey());
51-
}
52-
Assert.assertTrue("No thread with name " + entry.getKey() + " found.", entry.getValue());
53-
}
54-
55-
for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
56-
if (entry.getValue()) {
57-
LOG.info("Found thread for " + entry.getKey().getSimpleName());
58-
}
59-
Assert.assertTrue("No thread found for class " + entry.getKey().getSimpleName(),
60-
entry.getValue());
61-
}
38+
checkHouseKeepingThreadExistence(true);
6239
}
40+
6341
}

‎itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreHousekeepingNonLeader.java‎

Lines changed: 3 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,53 +19,23 @@
1919
package org.apache.hadoop.hive.metastore;
2020

2121
import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
22-
import org.apache.hadoop.hive.ql.txn.compactor.Worker;
23-
import org.junit.Assert;
2422
import org.junit.Before;
2523
import org.junit.Test;
26-
import org.slf4j.Logger;
27-
import org.slf4j.LoggerFactory;
28-
29-
import java.util.Map;
3024

3125
/**
3226
* Test for specifying HMS leader other than the current one.
3327
*/
3428
public class TestMetastoreHousekeepingNonLeader extends MetastoreHousekeepingLeaderTestBase {
35-
private static final Logger LOG =
36-
LoggerFactory.getLogger(TestMetastoreHousekeepingLeaderEmptyConfig.class);
3729

3830
@Before
3931
public void setUp() throws Exception {
4032
// Empty string for leader indicates that the HMS is leader.
41-
internalSetup("some_non_leader_host.domain1.domain", MetastoreConf.newMetastoreConf());
33+
setup("some_non_leader_host.domain1.domain", MetastoreConf.newMetastoreConf());
4234
}
4335

4436
@Test
4537
public void testHouseKeepingThreadExistence() throws Exception {
46-
searchHousekeepingThreads();
47-
48-
// Verify existence of threads
49-
for (Map.Entry<String, Boolean> entry : threadNames.entrySet()) {
50-
if (!entry.getValue()) {
51-
LOG.info("No thread found with name " + entry.getKey());
52-
}
53-
Assert.assertFalse("Thread with name " + entry.getKey() + " found.", entry.getValue());
54-
}
55-
56-
for (Map.Entry<Class<? extends Thread>, Boolean> entry : threadClasses.entrySet()) {
57-
// A non-leader HMS will still run the configured number of Compaction worker threads.
58-
if (entry.getKey() == Worker.class) {
59-
if (entry.getValue()) {
60-
LOG.info("Thread found for " + entry.getKey().getSimpleName());
61-
}
62-
} else {
63-
if (!entry.getValue()) {
64-
LOG.info("No thread found for " + entry.getKey().getSimpleName());
65-
}
66-
Assert.assertFalse("Thread found for class " + entry.getKey().getSimpleName(),
67-
entry.getValue());
68-
}
69-
}
38+
checkHouseKeepingThreadExistence(false);
7039
}
40+
7141
}

‎itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreLeaseLeader.java‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,20 @@
3434
import static org.junit.Assert.assertFalse;
3535
import static org.junit.Assert.assertTrue;
3636

37-
public class TestMetastoreLeaseLeader {
37+
public class TestMetastoreLeaseLeader extends MetastoreHousekeepingLeaderTestBase {
3838

3939
LeaderElection<TableName> election;
4040

41-
TestMetastoreHousekeepingLeader hms;
42-
4341
@Before
4442
public void setUp() throws Exception {
4543
Configuration configuration = MetastoreConf.newMetastoreConf();
46-
hms = new TestMetastoreHousekeepingLeader();
4744
MetastoreConf.setTimeVar(configuration, MetastoreConf.ConfVars.TXN_TIMEOUT, 1, TimeUnit.SECONDS);
4845
MetastoreConf.setTimeVar(configuration, MetastoreConf.ConfVars.LOCK_SLEEP_BETWEEN_RETRIES, 200, TimeUnit.MILLISECONDS);
4946
configuration.setBoolean(LeaseLeaderElection.METASTORE_RENEW_LEASE, false);
5047
configuration.setBoolean(LeaderElectionContext.LEADER_IN_TEST, true);
5148
configuration.set("hive.txn.manager", "org.apache.hadoop.hive.ql.lockmgr.DbTxnManager");
52-
hms.internalSetup(null, configuration);
49+
50+
setup(null, configuration);
5351

5452
Configuration conf = new Configuration(configuration);
5553
conf.setBoolean(LeaseLeaderElection.METASTORE_RENEW_LEASE, true);
@@ -65,28 +63,25 @@ public void testHouseKeepingThreads() throws Exception {
6563
CountDownLatch latch = new CountDownLatch(1);
6664
MetastoreHousekeepingLeaderTestBase.TestLeaderNotification.setMonitor(latch);
6765
// hms is the leader now
68-
hms.testHouseKeepingThreadExistence();
66+
checkHouseKeepingThreadExistence(true);
6967
assertFalse(election.isLeader());
7068
latch.await();
7169
// the lease of hms is timeout, election becomes leader now
7270
assertTrue(election.isLeader());
73-
try {
74-
// hms should shutdown all housekeeping tasks
75-
hms.testHouseKeepingThreadExistence();
76-
throw new IllegalStateException("HMS should shutdown all housekeeping tasks");
77-
} catch (AssertionError e) {
78-
// expected
79-
}
71+
// hms should shutdown all housekeeping tasks
72+
checkHouseKeepingThreadExistence(false);
73+
8074
latch = new CountDownLatch(1);
8175
MetastoreHousekeepingLeaderTestBase.TestLeaderNotification.setMonitor(latch);
8276
election.close();
8377
latch.await();
8478
// hms becomes leader again
85-
hms.testHouseKeepingThreadExistence();
79+
checkHouseKeepingThreadExistence(true);
8680
}
8781

8882
@After
8983
public void afterTest() {
9084
MetastoreHousekeepingLeaderTestBase.TestLeaderNotification.reset();
9185
}
86+
9287
}

‎itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetastoreLeaseNonLeader.java‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,10 @@
3434

3535
import static org.junit.Assert.assertTrue;
3636

37-
public class TestMetastoreLeaseNonLeader {
37+
public class TestMetastoreLeaseNonLeader extends MetastoreHousekeepingLeaderTestBase {
3838

3939
LeaderElection<TableName> election;
4040

41-
TestMetastoreHousekeepingLeader hms;
42-
4341
@Before
4442
public void setUp() throws Exception {
4543
Configuration conf = MetastoreConf.newMetastoreConf();
@@ -53,32 +51,27 @@ public void setUp() throws Exception {
5351
election.tryBeLeader(conf, tableName);
5452
assertTrue("The elector should hold the lease now", election.isLeader());
5553
// start the non-leader hms now
56-
hms = new TestMetastoreHousekeepingLeader();
5754
Configuration configuration = new Configuration(conf);
5855
MetastoreConf.setTimeVar(configuration, MetastoreConf.ConfVars.LOCK_SLEEP_BETWEEN_RETRIES, 1, TimeUnit.SECONDS);
5956
configuration.setBoolean(LeaderElectionContext.LEADER_IN_TEST, true);
60-
hms.internalSetup(null, configuration);
57+
setup(null, configuration);
6158
}
6259

6360
@Test
6461
public void testHouseKeepingThreads() throws Exception {
65-
try {
66-
hms.testHouseKeepingThreadExistence();
67-
throw new IllegalStateException("HMS shouldn't start any housekeeping tasks");
68-
} catch (AssertionError e) {
69-
// expected
70-
}
62+
checkHouseKeepingThreadExistence(false);
7163
// elector releases the lease
7264
CountDownLatch latch = new CountDownLatch(1);
7365
MetastoreHousekeepingLeaderTestBase.TestLeaderNotification.setMonitor(latch);
7466
election.close();
7567
latch.await();
7668
// housing threads are here now as the hms wins the election
77-
hms.testHouseKeepingThreadExistence();
69+
checkHouseKeepingThreadExistence(true);
7870
}
7971

8072
@After
8173
public void afterTest() {
8274
MetastoreHousekeepingLeaderTestBase.TestLeaderNotification.reset();
8375
}
76+
8477
}

0 commit comments

Comments
 (0)