iamsanjay commented on code in PR #13037:
URL: https://github.com/apache/lucene/pull/13037#discussion_r1473877541
##########
lucene/core/src/test/org/apache/lucene/index/TestDirectoryReader.java:
##########
@@ -1005,55 +1010,59 @@ public void testTryIncRef() throws IOException {
dir.close();
}
- public void testStressTryIncRef() throws IOException, InterruptedException {
+ public void testStressTryIncRef() throws Exception {
Directory dir = newDirectory();
IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new
MockAnalyzer(random())));
- writer.addDocument(new Document());
- writer.commit();
- DirectoryReader r = DirectoryReader.open(dir);
- int numThreads = atLeast(2);
+ final ScheduledExecutorService closeReaderBeforeJoiningThreads =
+ Executors.newSingleThreadScheduledExecutor(new
NamedThreadFactory("testStressTryIncRef"));
- IncThread[] threads = new IncThread[numThreads];
- for (int i = 0; i < threads.length; i++) {
- threads[i] = new IncThread(r, random());
- threads[i].start();
- }
- Thread.sleep(100);
-
- assertTrue(r.tryIncRef());
- r.decRef();
- r.close();
-
- for (IncThread thread : threads) {
- thread.join();
- assertNull(thread.failed);
- }
- assertFalse(r.tryIncRef());
- writer.close();
- dir.close();
- }
-
- static class IncThread extends Thread {
- final IndexReader toInc;
- final Random random;
- Throwable failed;
-
- IncThread(IndexReader toInc, Random random) {
- this.toInc = toInc;
- this.random = random;
- }
Review Comment:
I refactored `testStressTryIncRef` test bit further, removed the IncThread
class and convert it into a runnable. And with that remove the call to
`random()` which we do not use anymore.
Also introduce the try-with-resources statement. I would love to get some
feedback on it!
I observed that usually we are using `Thread.sleep`, in many cases, where we
want to delay the execution of some lines of code in the main thread. For
instance, closing IndexReader in above case before stress test the
`tryIncRef()` and `decRef()` via running two threads simultaneously. To replace
`Thread.sleep`, I inspect the code for which we want the delayed execution and
then schedule that block via `ScheduledExecutorService`. Having said that, I
have my suspicion that whether we want to pivot in that direction.
--
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...@lucene.apache.org
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
