Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import static java.util.concurrent.Executors.newFixedThreadPool;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.testng.Assert.assertTrue;

public class TestEmptyCache
{
Expand Down Expand Up @@ -80,7 +81,7 @@ public void testLoadFailure()
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ public void testLoadFailure()
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down Expand Up @@ -407,7 +407,7 @@ public void testConcurrentGetWithCallableShareLoad()
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down Expand Up @@ -473,7 +473,7 @@ public void testInvalidateOngoingLoad(Invalidation invalidation)
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down Expand Up @@ -543,7 +543,7 @@ public void testInvalidateAndLoadConcurrently(Invalidation invalidation)
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ public void testConcurrentGetWithCallableShareLoad()
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down Expand Up @@ -521,7 +521,7 @@ public Integer load(Integer key)
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down Expand Up @@ -599,7 +599,7 @@ public String load(Integer key)
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down Expand Up @@ -672,7 +672,7 @@ public void testInvalidateAndLoadConcurrently(Invalidation invalidation)
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ public Map<String, Optional<Partition>> getPartitionsByNames(Table table, List<S
getPartitionsByNamesFinishedLatch.countDown();

executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public void testDeleteRowsConcurrently()
}
finally {
executor.shutdownNow();
executor.awaitTermination(10, SECONDS);
assertTrue(executor.awaitTermination(10, SECONDS));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import java.util.concurrent.ExecutorCompletionService;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
Expand Down Expand Up @@ -1947,7 +1948,8 @@ protected void testReadMetadataWithRelationsConcurrentModifications(int readIter
writeTasksCount = 2 * writeTasksCount; // writes are scheduled twice
CountDownLatch writeTasksInitialized = new CountDownLatch(writeTasksCount);
Runnable writeInitialized = writeTasksInitialized::countDown;
Supplier<Boolean> done = () -> incompleteReadTasks.get() == 0;
AtomicBoolean aborted = new AtomicBoolean();
Supplier<Boolean> done = () -> aborted.get() || incompleteReadTasks.get() == 0;
List<Callable<Void>> writeTasks = new ArrayList<>();
writeTasks.add(createDropRepeatedly(writeInitialized, done, "concur_table", createTableSqlTemplateForConcurrentModifications(), "DROP TABLE %s"));
if (hasBehavior(SUPPORTS_CREATE_VIEW)) {
Expand Down Expand Up @@ -1978,6 +1980,14 @@ protected void testReadMetadataWithRelationsConcurrentModifications(int readIter
future.get(); // non-blocking
}
}
catch (Throwable failure) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message is Ensure test threads termination even when test overridden - feels like a general change; but it is for specific test only.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait - did you miss throw failure here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait - did you miss throw failure here?

good catch!

The commit message is Ensure test threads termination even when test overridden - feels like a general change; but it is for specific test only.

yes, but i couldn't put testReadMetadataWithRelationsConcurrentModifications in the title line, could i?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the following text was misleading to me. It was stating testReadMetadataWithRelationsConcurrentModifications as an "example" - which I interpreted as the change is more generic, and you are explaining why it is helpful using this specifi testcase.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that i have read the commit message, i see your point. reworded

aborted.set(true);
executor.shutdownNow();
if (!executor.awaitTermination(10, SECONDS)) {
throw new AssertionError("Test threads did not complete. Leaving test threads behind may violate AbstractTestQueryFramework.checkQueryInfosFinal", failure);
}
throw failure;
}
finally {
executor.shutdownNow();
}
Expand Down