You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by jo...@apache.org on 2018/10/11 01:02:15 UTC
[incubator-druid] branch master updated: fixes for
LookupReferencesManagerTest (#6444)
This is an automated email from the ASF dual-hosted git repository.
jonwei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-druid.git
The following commit(s) were added to refs/heads/master by this push:
new f7775d1 fixes for LookupReferencesManagerTest (#6444)
f7775d1 is described below
commit f7775d1db32b2adff2fb457b7e3d7a04a993a2b4
Author: Clint Wylie <cj...@gmail.com>
AuthorDate: Wed Oct 10 18:02:11 2018 -0700
fixes for LookupReferencesManagerTest (#6444)
* some fixes for LookupReferencesManagerTest
* docs
* formatting
* more formatting fixes
---
docs/content/querying/lookups.md | 1 +
.../apache/druid/query/lookup/LookupConfig.java | 20 ++-
.../druid/query/lookup/LookupConfigTest.java | 4 +-
.../query/lookup/LookupReferencesManager.java | 10 +-
.../query/lookup/LookupReferencesManagerTest.java | 134 ++++++++++++++++-----
5 files changed, 131 insertions(+), 38 deletions(-)
diff --git a/docs/content/querying/lookups.md b/docs/content/querying/lookups.md
index 17dcef9..479376b 100644
--- a/docs/content/querying/lookups.md
+++ b/docs/content/querying/lookups.md
@@ -361,6 +361,7 @@ It is possible to save the configuration across restarts such that a node will n
|`druid.lookup.numLookupLoadingThreads`|Number of threads for loading the lookups in parallel on startup. This thread pool is destroyed once startup is done. It is not kept during the lifetime of the JVM|Available Processors / 2|
|`druid.lookup.coordinatorFetchRetries`|How many times to retry to fetch the lookup bean list from coordinator, during the sync on startup.|3|
|`druid.lookup.lookupStartRetries`|How many times to retry to start each lookup, either during the sync on startup, or during the runtime.|3|
+|`druid.lookup.coordinatorRetryDelay`|How long to delay (in millis) between retries to fetch lookup list from the coordinator during the sync on startup.|60_000|
## Introspect a Lookup
diff --git a/processing/src/main/java/org/apache/druid/query/lookup/LookupConfig.java b/processing/src/main/java/org/apache/druid/query/lookup/LookupConfig.java
index fd2dc84..6746e08 100644
--- a/processing/src/main/java/org/apache/druid/query/lookup/LookupConfig.java
+++ b/processing/src/main/java/org/apache/druid/query/lookup/LookupConfig.java
@@ -28,6 +28,7 @@ import java.util.Objects;
public class LookupConfig
{
+ static int DEFAULT_COORDINATOR_RETRY_DELAY = 60_000;
@JsonProperty("snapshotWorkingDir")
private String snapshotWorkingDir;
@@ -43,6 +44,13 @@ public class LookupConfig
@JsonProperty("coordinatorFetchRetries")
private int coordinatorFetchRetries = 3;
+ // By default, add an extra minute in addition to the retry wait. In RetryUtils, retry wait starts from a few
+ // seconds, that is likely not enough to coordinator to be back to healthy state, e. g. if it experiences
+ // 30-second GC pause.
+ @Min(0)
+ @JsonProperty("coordinatorRetryDelay")
+ private int coordinatorRetryDelay = DEFAULT_COORDINATOR_RETRY_DELAY;
+
@Min(1)
@JsonProperty("lookupStartRetries")
private int lookupStartRetries = 3;
@@ -84,6 +92,11 @@ public class LookupConfig
return lookupStartRetries;
}
+ public int getCoordinatorRetryDelay()
+ {
+ return coordinatorRetryDelay;
+ }
+
@Override
public boolean equals(Object o)
{
@@ -100,7 +113,8 @@ public class LookupConfig
enableLookupSyncOnStartup == that.enableLookupSyncOnStartup &&
numLookupLoadingThreads == that.numLookupLoadingThreads &&
coordinatorFetchRetries == that.coordinatorFetchRetries &&
- lookupStartRetries == that.lookupStartRetries;
+ lookupStartRetries == that.lookupStartRetries &&
+ coordinatorRetryDelay == that.coordinatorRetryDelay;
}
@Override
@@ -111,7 +125,8 @@ public class LookupConfig
enableLookupSyncOnStartup,
numLookupLoadingThreads,
coordinatorFetchRetries,
- lookupStartRetries
+ lookupStartRetries,
+ coordinatorRetryDelay
);
}
@@ -124,6 +139,7 @@ public class LookupConfig
", numLookupLoadingThreads=" + numLookupLoadingThreads +
", coordinatorFetchRetries=" + coordinatorFetchRetries +
", lookupStartRetries=" + lookupStartRetries +
+ ", coordinatorRetryDelay=" + coordinatorRetryDelay +
'}';
}
}
diff --git a/processing/src/test/java/org/apache/druid/query/lookup/LookupConfigTest.java b/processing/src/test/java/org/apache/druid/query/lookup/LookupConfigTest.java
index 6c268d9..ca18913 100644
--- a/processing/src/test/java/org/apache/druid/query/lookup/LookupConfigTest.java
+++ b/processing/src/test/java/org/apache/druid/query/lookup/LookupConfigTest.java
@@ -53,7 +53,8 @@ public class LookupConfigTest
+ " \"snapshotWorkingDir\": \"/tmp\",\n"
+ " \"numLookupLoadingThreads\": 4,\n"
+ " \"coordinatorFetchRetries\": 4,\n"
- + " \"lookupStartRetries\": 4 \n"
+ + " \"lookupStartRetries\": 4,\n"
+ + " \"coordinatorRetryDelay\": 100 \n"
+ "}\n";
LookupConfig config = mapper.readValue(
mapper.writeValueAsString(
@@ -67,5 +68,6 @@ public class LookupConfigTest
Assert.assertEquals(4, config.getNumLookupLoadingThreads());
Assert.assertEquals(4, config.getCoordinatorFetchRetries());
Assert.assertEquals(4, config.getLookupStartRetries());
+ Assert.assertEquals(100, config.getCoordinatorRetryDelay());
}
}
diff --git a/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java b/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java
index 8d97cbe..6a6ca6c 100644
--- a/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java
+++ b/server/src/main/java/org/apache/druid/query/lookup/LookupReferencesManager.java
@@ -389,11 +389,11 @@ public class LookupReferencesManager
() -> {
if (firstAttempt.isTrue()) {
firstAttempt.setValue(false);
- } else {
- // Adding an extra minute in addition to the retry wait. In RetryUtils, retry wait starts from a few
- // seconds, that is likely not enough to coordinator to be back to healthy state, e. g. if it experiences
- // 30-second GC pause.
- Thread.sleep(60_000);
+ } else if (lookupConfig.getCoordinatorRetryDelay() > 0) {
+ // Adding any configured extra time in addition to the retry wait. In RetryUtils, retry wait starts from
+ // a few seconds, that is likely not enough to coordinator to be back to healthy state, e. g. if it
+ // experiences 30-second GC pause. Default is 1 minute
+ Thread.sleep(lookupConfig.getCoordinatorRetryDelay());
}
return tryGetLookupListFromCoordinator(tier);
},
diff --git a/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java b/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java
index f155895..5c0f168 100644
--- a/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java
+++ b/server/src/test/java/org/apache/druid/query/lookup/LookupReferencesManagerTest.java
@@ -50,20 +50,15 @@ import static org.easymock.EasyMock.reset;
public class LookupReferencesManagerTest
{
- LookupReferencesManager lookupReferencesManager;
-
- private DruidLeaderClient druidLeaderClient;
-
- private LookupListeningAnnouncerConfig config;
-
private static final String LOOKUP_TIER = "lookupTier";
-
- LookupExtractorFactory lookupExtractorFactory;
-
- LookupExtractorFactoryContainer container;
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ LookupReferencesManager lookupReferencesManager;
+ LookupExtractorFactory lookupExtractorFactory;
+ LookupExtractorFactoryContainer container;
ObjectMapper mapper = new DefaultObjectMapper();
+ private DruidLeaderClient druidLeaderClient;
+ private LookupListeningAnnouncerConfig config;
@Before
public void setUp() throws IOException
@@ -85,7 +80,9 @@ public class LookupReferencesManagerTest
String temporaryPath = temporaryFolder.newFolder().getAbsolutePath();
lookupReferencesManager = new LookupReferencesManager(
new LookupConfig(temporaryFolder.newFolder().getAbsolutePath()),
- mapper, druidLeaderClient, config,
+ mapper,
+ druidLeaderClient,
+ config,
true
);
}
@@ -104,7 +101,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -165,7 +165,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -203,7 +206,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -234,7 +240,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -262,7 +271,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -292,7 +304,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -326,7 +341,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -354,7 +372,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -405,7 +426,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -447,7 +471,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -523,7 +550,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -543,13 +573,31 @@ public class LookupReferencesManagerTest
@Test
public void testLoadLookupOnCoordinatorFailure() throws Exception
{
+ LookupConfig lookupConfig = new LookupConfig(temporaryFolder.newFolder().getAbsolutePath())
+ {
+ @Override
+ public int getCoordinatorRetryDelay()
+ {
+ return 10;
+ }
+ };
+ lookupReferencesManager = new LookupReferencesManager(
+ lookupConfig,
+ mapper,
+ druidLeaderClient,
+ config
+ );
+
Map<String, Object> lookupMap = new HashMap<>();
lookupMap.put("testMockForLoadLookupOnCoordinatorFailure", container);
String strResult = mapper.writeValueAsString(lookupMap);
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request)
.anyTimes();
FullResponseHolder responseHolder = new FullResponseHolder(
@@ -564,16 +612,30 @@ public class LookupReferencesManagerTest
lookupReferencesManager.add("testMockForLoadLookupOnCoordinatorFailure", container);
lookupReferencesManager.handlePendingNotices();
lookupReferencesManager.stop();
+ lookupConfig = new LookupConfig(lookupReferencesManager.lookupSnapshotTaker.getPersistFile(LOOKUP_TIER).getParent())
+ {
+ @Override
+ public int getCoordinatorRetryDelay()
+ {
+ return 10;
+ }
+ };
+
lookupReferencesManager = new LookupReferencesManager(
- new LookupConfig(lookupReferencesManager.lookupSnapshotTaker.getPersistFile(LOOKUP_TIER).getParent()),
- mapper, druidLeaderClient, config,
+ lookupConfig,
+ mapper,
+ druidLeaderClient,
+ config,
true
);
reset(config);
reset(druidLeaderClient);
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request)
.anyTimes();
expect(druidLeaderClient.go(request)).andThrow(new IllegalStateException()).anyTimes();
@@ -585,9 +647,19 @@ public class LookupReferencesManagerTest
@Test
public void testDisableLookupSync() throws Exception
{
+ LookupConfig lookupConfig = new LookupConfig(null)
+ {
+ @Override
+ public boolean getEnableLookupSyncOnStartup()
+ {
+ return false;
+ }
+ };
LookupReferencesManager lookupReferencesManager = new LookupReferencesManager(
- new LookupConfig(null),
- mapper, druidLeaderClient, config
+ lookupConfig,
+ mapper,
+ druidLeaderClient,
+ config
);
Map<String, Object> lookupMap = new HashMap<>();
lookupMap.put("testMockForDisableLookupSync", container);
@@ -595,7 +667,10 @@ public class LookupReferencesManagerTest
Request request = new Request(HttpMethod.GET, new URL("http://localhost:1234/xx"));
expect(config.getLookupTier()).andReturn(LOOKUP_TIER).anyTimes();
replay(config);
- expect(druidLeaderClient.makeRequest(HttpMethod.GET, "/druid/coordinator/v1/lookups/lookupTier?detailed=true"))
+ expect(druidLeaderClient.makeRequest(
+ HttpMethod.GET,
+ "/druid/coordinator/v1/lookups/config/lookupTier?detailed=true"
+ ))
.andReturn(request);
FullResponseHolder responseHolder = new FullResponseHolder(
HttpResponseStatus.OK,
@@ -607,5 +682,4 @@ public class LookupReferencesManagerTest
lookupReferencesManager.start();
Assert.assertNull(lookupReferencesManager.get("testMockForDisableLookupSync"));
}
-
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org