dweiss commented on a change in pull request #1842:
URL: https://github.com/apache/lucene-solr/pull/1842#discussion_r485116034
##########
File path: gradle/testing/randomization/policies/tests.policy
##########
@@ -64,6 +64,9 @@ grant {
permission java.lang.RuntimePermission "getClassLoader";
permission java.lang.RuntimePermission "setContextClassLoader";
+ // TestLockFactoriesMultiJVM opens a random port on 127.0.0.1 (port 0 =
ephemeral port range):
Review comment:
This property wouldn't be needed if you forked the "server: side as well
as clients... :)
##########
File path:
lucene/core/src/test/org/apache/lucene/store/TestLockFactoriesMultiJVM.java
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.store;
+
+import java.lang.ProcessBuilder.Redirect;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.NamedThreadFactory;
+import org.apache.lucene.util.SuppressForbidden;
+
+public class TestLockFactoriesMultiJVM extends LuceneTestCase {
+
+ @SuppressForbidden(reason = "ProcessBuilder only allows to redirect to
java.io.File")
+ private static final ProcessBuilder applyRedirection(ProcessBuilder pb, int
client, Path dir) {
+ if (LuceneTestCase.VERBOSE) {
+ return pb.inheritIO();
+ } else {
+ return pb
+ .redirectError(dir.resolve("err-" + client + ".txt").toFile())
+ .redirectOutput(dir.resolve("out-" + client + ".txt").toFile())
+ .redirectInput(Redirect.INHERIT);
+ }
+ }
+
+ private void runImpl(Class<? extends LockFactory> impl) throws Exception {
+ // make sure we are in clean state:
+ LockVerifyServer.PORT.set(-1);
+
+ final int clients = 2;
+ final String host = "127.0.0.1";
+ final int delay = 1;
+ final int rounds = (LuceneTestCase.TEST_NIGHTLY ? 30000 : 500) *
LuceneTestCase.RANDOM_MULTIPLIER;
+
+ final Path dir = LuceneTestCase.createTempDir(impl.getSimpleName());
+
+ // create the LockVerifyServer in a separate thread
+ final ExecutorService pool = Executors.newSingleThreadExecutor(new
NamedThreadFactory("lockfactory-tester-"));
+ try {
+ pool.submit(() -> {
+ LockVerifyServer.main(host, Integer.toString(clients));
+ return (Void) null;
+ });
+
+ // wait for it to boot up
+ int port;
+ while ((port = LockVerifyServer.PORT.get()) == -1) {
+ Thread.sleep(100L);
+ }
+
+ // spawn clients as separate Java processes
+ final List<Process> processes = new ArrayList<>();
+ for (int i = 0; i < clients; i++) {
+ processes.add(applyRedirection(new ProcessBuilder(
+ Paths.get(System.getProperty("java.home"), "bin",
"java").toString(),
+ "-cp",
+ System.getProperty("java.class.path"),
+ LockStressTest.class.getName(),
+ Integer.toString(i),
+ host,
+ Integer.toString(port),
+ impl.getName(),
+ dir.toString(),
+ Integer.toString(delay),
+ Integer.toString(rounds)
+ ), i, dir).start());
+ }
+
+ // wait for all processes to exit
+ int exited = 0;
+ while (exited < clients) {
+ for (Process p : processes) {
+ if (p.waitFor(1, TimeUnit.SECONDS)) {
Review comment:
1 second may not be enough on heavy loaded machines?
##########
File path:
lucene/core/src/test/org/apache/lucene/store/TestLockFactoriesMultiJVM.java
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.store;
+
+import java.lang.ProcessBuilder.Redirect;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.NamedThreadFactory;
+import org.apache.lucene.util.SuppressForbidden;
+
+public class TestLockFactoriesMultiJVM extends LuceneTestCase {
+
+ @SuppressForbidden(reason = "ProcessBuilder only allows to redirect to
java.io.File")
+ private static final ProcessBuilder applyRedirection(ProcessBuilder pb, int
client, Path dir) {
+ if (LuceneTestCase.VERBOSE) {
+ return pb.inheritIO();
+ } else {
+ return pb
+ .redirectError(dir.resolve("err-" + client + ".txt").toFile())
+ .redirectOutput(dir.resolve("out-" + client + ".txt").toFile())
+ .redirectInput(Redirect.INHERIT);
+ }
+ }
+
+ private void runImpl(Class<? extends LockFactory> impl) throws Exception {
+ // make sure we are in clean state:
+ LockVerifyServer.PORT.set(-1);
+
+ final int clients = 2;
+ final String host = "127.0.0.1";
+ final int delay = 1;
+ final int rounds = (LuceneTestCase.TEST_NIGHTLY ? 30000 : 500) *
LuceneTestCase.RANDOM_MULTIPLIER;
+
+ final Path dir = LuceneTestCase.createTempDir(impl.getSimpleName());
+
+ // create the LockVerifyServer in a separate thread
+ final ExecutorService pool = Executors.newSingleThreadExecutor(new
NamedThreadFactory("lockfactory-tester-"));
+ try {
+ pool.submit(() -> {
+ LockVerifyServer.main(host, Integer.toString(clients));
+ return (Void) null;
+ });
+
+ // wait for it to boot up
+ int port;
+ while ((port = LockVerifyServer.PORT.get()) == -1) {
+ Thread.sleep(100L);
+ }
+
+ // spawn clients as separate Java processes
+ final List<Process> processes = new ArrayList<>();
+ for (int i = 0; i < clients; i++) {
+ processes.add(applyRedirection(new ProcessBuilder(
+ Paths.get(System.getProperty("java.home"), "bin",
"java").toString(),
+ "-cp",
+ System.getProperty("java.class.path"),
+ LockStressTest.class.getName(),
+ Integer.toString(i),
+ host,
+ Integer.toString(port),
+ impl.getName(),
+ dir.toString(),
+ Integer.toString(delay),
+ Integer.toString(rounds)
+ ), i, dir).start());
+ }
+
+ // wait for all processes to exit
+ int exited = 0;
+ while (exited < clients) {
+ for (Process p : processes) {
Review comment:
I think we should absolutely try to clean up forked processes in case
something goes wrong (even if it's an internal suite timeout!) - regardless of
the outcome of the test. I'd do a run over the process list in finally and if
it's still in a live state, kill it.
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Process.html#destroyForcibly()
Another thought is maybe to use a list of completable futures to wait for
all forked processes instead of counting them?
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Process.html#onExit()
These are details; the cleanup is kind of important.
##########
File path:
lucene/core/src/test/org/apache/lucene/store/TestLockFactoriesMultiJVM.java
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.store;
+
+import java.lang.ProcessBuilder.Redirect;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.NamedThreadFactory;
+import org.apache.lucene.util.SuppressForbidden;
+
+public class TestLockFactoriesMultiJVM extends LuceneTestCase {
+
+ @SuppressForbidden(reason = "ProcessBuilder only allows to redirect to
java.io.File")
+ private static final ProcessBuilder applyRedirection(ProcessBuilder pb, int
client, Path dir) {
+ if (LuceneTestCase.VERBOSE) {
+ return pb.inheritIO();
+ } else {
+ return pb
+ .redirectError(dir.resolve("err-" + client + ".txt").toFile())
+ .redirectOutput(dir.resolve("out-" + client + ".txt").toFile())
+ .redirectInput(Redirect.INHERIT);
+ }
+ }
+
+ private void runImpl(Class<? extends LockFactory> impl) throws Exception {
+ // make sure we are in clean state:
+ LockVerifyServer.PORT.set(-1);
+
+ final int clients = 2;
+ final String host = "127.0.0.1";
+ final int delay = 1;
+ final int rounds = (LuceneTestCase.TEST_NIGHTLY ? 30000 : 500) *
LuceneTestCase.RANDOM_MULTIPLIER;
+
+ final Path dir = LuceneTestCase.createTempDir(impl.getSimpleName());
+
+ // create the LockVerifyServer in a separate thread
+ final ExecutorService pool = Executors.newSingleThreadExecutor(new
NamedThreadFactory("lockfactory-tester-"));
+ try {
+ pool.submit(() -> {
+ LockVerifyServer.main(host, Integer.toString(clients));
+ return (Void) null;
+ });
+
+ // wait for it to boot up
+ int port;
+ while ((port = LockVerifyServer.PORT.get()) == -1) {
+ Thread.sleep(100L);
+ }
+
+ // spawn clients as separate Java processes
+ final List<Process> processes = new ArrayList<>();
+ for (int i = 0; i < clients; i++) {
+ processes.add(applyRedirection(new ProcessBuilder(
+ Paths.get(System.getProperty("java.home"), "bin",
"java").toString(),
Review comment:
Does it make sense to limit the default heap of these subprocesses here?
##########
File path:
lucene/core/src/test/org/apache/lucene/store/TestLockFactoriesMultiJVM.java
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.store;
+
+import java.lang.ProcessBuilder.Redirect;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.lucene.util.LuceneTestCase;
+import org.apache.lucene.util.NamedThreadFactory;
+import org.apache.lucene.util.SuppressForbidden;
+
+public class TestLockFactoriesMultiJVM extends LuceneTestCase {
Review comment:
I would maybe mark this as belonging to Slow group? Just a thought.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
