madrob commented on code in PR #829:
URL: https://github.com/apache/solr/pull/829#discussion_r862210736
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -51,39 +60,50 @@ protected String getCloudSolrConfig() {
return "solrconfig-managed-schema.xml";
}
+ @Before
+ public void setup() {
+ super.beforeTest();
Review Comment:
I don't think we need to call this since we're not actually overriding the
method? I don't see the other tests that extend AFDZTC doing it.
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -51,39 +60,50 @@ protected String getCloudSolrConfig() {
return "solrconfig-managed-schema.xml";
}
+ @Before
+ public void setup() {
+ super.beforeTest();
+ setupRestTestHarnesses();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ closeRestTestHarnesses();
+ super.tearDown();
+ }
+
@Test
- @SuppressWarnings({"unchecked"})
public void test() throws Exception {
+ final List<List<String>> collectErrors = Collections.synchronizedList(new
ArrayList<>());
- final int threadCount = 5;
- setupRestTestHarnesses();
- Thread[] threads = new Thread[threadCount];
- @SuppressWarnings({"rawtypes"})
- final List<List> collectErrors = Collections.synchronizedList(new
ArrayList<>());
+ final ExecutorService executorService =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ THREAD_COUNT, new
SolrNamedThreadFactory(this.getClass().getSimpleName()));
- for (int i = 0; i < threadCount; i++) {
+ List<Callable<Void>> callees = new ArrayList<>(THREAD_COUNT);
+ for (int i = 0; i < THREAD_COUNT; i++) {
final int finalI = i;
- threads[i] =
- new Thread() {
- @Override
- public void run() {
- @SuppressWarnings({"rawtypes"})
- ArrayList errs = new ArrayList();
- collectErrors.add(errs);
- try {
- invokeBulkAddCall(finalI, errs);
- invokeBulkReplaceCall(finalI, errs);
- invokeBulkDeleteCall(finalI, errs);
- } catch (Exception e) {
- e.printStackTrace();
- }
+ Callable<Void> call =
+ () -> {
+ List<String> errs = new ArrayList<>();
+ collectErrors.add(errs);
+ try {
+ invokeBulkAddCall(finalI, errs);
+ invokeBulkReplaceCall(finalI, errs);
+ invokeBulkDeleteCall(finalI, errs);
+ } catch (Exception e) {
+ e.printStackTrace();
Review Comment:
delete this since we would log it later?
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -51,39 +60,50 @@ protected String getCloudSolrConfig() {
return "solrconfig-managed-schema.xml";
}
+ @Before
+ public void setup() {
+ super.beforeTest();
+ setupRestTestHarnesses();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ closeRestTestHarnesses();
+ super.tearDown();
+ }
+
@Test
- @SuppressWarnings({"unchecked"})
public void test() throws Exception {
+ final List<List<String>> collectErrors = Collections.synchronizedList(new
ArrayList<>());
- final int threadCount = 5;
- setupRestTestHarnesses();
- Thread[] threads = new Thread[threadCount];
- @SuppressWarnings({"rawtypes"})
- final List<List> collectErrors = Collections.synchronizedList(new
ArrayList<>());
+ final ExecutorService executorService =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ THREAD_COUNT, new
SolrNamedThreadFactory(this.getClass().getSimpleName()));
- for (int i = 0; i < threadCount; i++) {
+ List<Callable<Void>> callees = new ArrayList<>(THREAD_COUNT);
+ for (int i = 0; i < THREAD_COUNT; i++) {
final int finalI = i;
- threads[i] =
- new Thread() {
- @Override
- public void run() {
- @SuppressWarnings({"rawtypes"})
- ArrayList errs = new ArrayList();
- collectErrors.add(errs);
- try {
- invokeBulkAddCall(finalI, errs);
- invokeBulkReplaceCall(finalI, errs);
- invokeBulkDeleteCall(finalI, errs);
- } catch (Exception e) {
- e.printStackTrace();
- }
+ Callable<Void> call =
+ () -> {
+ List<String> errs = new ArrayList<>();
+ collectErrors.add(errs);
+ try {
+ invokeBulkAddCall(finalI, errs);
+ invokeBulkReplaceCall(finalI, errs);
+ invokeBulkDeleteCall(finalI, errs);
+ } catch (Exception e) {
+ e.printStackTrace();
+ Thread.currentThread().interrupt();
}
+ return null;
};
-
- threads[i].start();
+ callees.add(call);
}
- for (Thread thread : threads) thread.join();
+ executorService.invokeAll(callees);
+ executorService.shutdown();
+ assertTrue(
+ "Running for too long...", executorService.awaitTermination(TIMEOUT *
3, TimeUnit.SECONDS));
Review Comment:
`TIMEOUT * 3` seems strange here?
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -340,9 +370,13 @@ private void invokeBulkDeleteCall(int seed,
ArrayList<String> errs) throws Excep
private boolean checkCopyField(
@SuppressWarnings({"rawtypes"}) List<Map> l, String src, String dest) {
- if (l == null) return false;
+ if (l == null) {
+ return false;
+ }
for (@SuppressWarnings({"rawtypes"}) Map map : l) {
Review Comment:
can we fix these raw types too?
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -145,46 +166,49 @@ private void invokeBulkAddCall(int seed,
ArrayList<String> errs) throws Exceptio
// get another node
Set<String> errmessages = new HashSet<>();
+ // don't close harness - gets closed at teardown
RestTestHarness harness = randomRestTestHarness(r);
- try {
- long startTime = System.nanoTime();
- long maxTimeoutMillis = 100000;
- while (TimeUnit.MILLISECONDS.convert(System.nanoTime() - startTime,
TimeUnit.NANOSECONDS)
- < maxTimeoutMillis) {
- errmessages.clear();
- @SuppressWarnings({"rawtypes"})
- Map m = getObj(harness, aField, "fields");
- if (m == null) errmessages.add(StrUtils.formatString("field {0} not
created", aField));
-
- m = getObj(harness, dynamicFldName, "dynamicFields");
- if (m == null)
- errmessages.add(StrUtils.formatString("dynamic field {0} not
created", dynamicFldName));
-
- @SuppressWarnings({"rawtypes"})
- List l = getSourceCopyFields(harness, aField);
- if (!checkCopyField(l, aField, dynamicCopyFldDest))
- errmessages.add(
- StrUtils.formatString(
- "CopyField source={0},dest={1} not created", aField,
dynamicCopyFldDest));
-
- m = getObj(harness, newFieldTypeName, "fieldTypes");
- if (m == null)
- errmessages.add(StrUtils.formatString("new type {0} not created",
newFieldTypeName));
-
- if (errmessages.isEmpty()) break;
-
- Thread.sleep(10);
+ long startTime = System.nanoTime();
Review Comment:
We can use a TimeOut here, might be a little clearer?
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -51,39 +60,50 @@ protected String getCloudSolrConfig() {
return "solrconfig-managed-schema.xml";
}
+ @Before
+ public void setup() {
+ super.beforeTest();
+ setupRestTestHarnesses();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ closeRestTestHarnesses();
+ super.tearDown();
+ }
+
@Test
- @SuppressWarnings({"unchecked"})
public void test() throws Exception {
+ final List<List<String>> collectErrors = Collections.synchronizedList(new
ArrayList<>());
- final int threadCount = 5;
- setupRestTestHarnesses();
- Thread[] threads = new Thread[threadCount];
- @SuppressWarnings({"rawtypes"})
- final List<List> collectErrors = Collections.synchronizedList(new
ArrayList<>());
+ final ExecutorService executorService =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ THREAD_COUNT, new
SolrNamedThreadFactory(this.getClass().getSimpleName()));
- for (int i = 0; i < threadCount; i++) {
+ List<Callable<Void>> callees = new ArrayList<>(THREAD_COUNT);
+ for (int i = 0; i < THREAD_COUNT; i++) {
final int finalI = i;
- threads[i] =
- new Thread() {
- @Override
- public void run() {
- @SuppressWarnings({"rawtypes"})
- ArrayList errs = new ArrayList();
- collectErrors.add(errs);
- try {
- invokeBulkAddCall(finalI, errs);
- invokeBulkReplaceCall(finalI, errs);
- invokeBulkDeleteCall(finalI, errs);
- } catch (Exception e) {
- e.printStackTrace();
- }
+ Callable<Void> call =
+ () -> {
+ List<String> errs = new ArrayList<>();
+ collectErrors.add(errs);
+ try {
+ invokeBulkAddCall(finalI, errs);
+ invokeBulkReplaceCall(finalI, errs);
+ invokeBulkDeleteCall(finalI, errs);
+ } catch (Exception e) {
+ e.printStackTrace();
+ Thread.currentThread().interrupt();
Review Comment:
Do we need this so that the executor will shut down properly on error?
Confused about why we interrupt on every exception, not just IE.
##########
solr/core/src/test/org/apache/solr/schema/TestBulkSchemaConcurrent.java:
##########
@@ -51,39 +60,50 @@ protected String getCloudSolrConfig() {
return "solrconfig-managed-schema.xml";
}
+ @Before
+ public void setup() {
+ super.beforeTest();
+ setupRestTestHarnesses();
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ closeRestTestHarnesses();
+ super.tearDown();
+ }
+
@Test
- @SuppressWarnings({"unchecked"})
public void test() throws Exception {
+ final List<List<String>> collectErrors = Collections.synchronizedList(new
ArrayList<>());
- final int threadCount = 5;
- setupRestTestHarnesses();
- Thread[] threads = new Thread[threadCount];
- @SuppressWarnings({"rawtypes"})
- final List<List> collectErrors = Collections.synchronizedList(new
ArrayList<>());
+ final ExecutorService executorService =
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ THREAD_COUNT, new
SolrNamedThreadFactory(this.getClass().getSimpleName()));
- for (int i = 0; i < threadCount; i++) {
+ List<Callable<Void>> callees = new ArrayList<>(THREAD_COUNT);
+ for (int i = 0; i < THREAD_COUNT; i++) {
final int finalI = i;
- threads[i] =
- new Thread() {
- @Override
- public void run() {
- @SuppressWarnings({"rawtypes"})
- ArrayList errs = new ArrayList();
- collectErrors.add(errs);
- try {
- invokeBulkAddCall(finalI, errs);
- invokeBulkReplaceCall(finalI, errs);
- invokeBulkDeleteCall(finalI, errs);
- } catch (Exception e) {
- e.printStackTrace();
- }
+ Callable<Void> call =
+ () -> {
+ List<String> errs = new ArrayList<>();
+ collectErrors.add(errs);
+ try {
+ invokeBulkAddCall(finalI, errs);
+ invokeBulkReplaceCall(finalI, errs);
+ invokeBulkDeleteCall(finalI, errs);
+ } catch (Exception e) {
+ e.printStackTrace();
+ Thread.currentThread().interrupt();
}
+ return null;
};
-
- threads[i].start();
+ callees.add(call);
}
- for (Thread thread : threads) thread.join();
+ executorService.invokeAll(callees);
+ executorService.shutdown();
+ assertTrue(
+ "Running for too long...", executorService.awaitTermination(TIMEOUT *
3, TimeUnit.SECONDS));
boolean success = true;
Review Comment:
Next line: please fix the raw types.
--
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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
