virajjasani commented on a change in pull request #982:
URL: https://github.com/apache/phoenix/pull/982#discussion_r530623628
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
##########
@@ -104,10 +111,30 @@ public void testUpdateCacheForNeverUpdatedTable() throws
Exception {
String tableName = generateUniqueName();
String fullTableName = INDEX_DATA_SCHEMA +
QueryConstants.NAME_SEPARATOR + tableName;
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ AtomicBoolean isSysMutexEmpty = new AtomicBoolean(true);
+ ExecutorService executorService = Executors.newFixedThreadPool(5,
+ new ThreadFactoryBuilder().setDaemon(true)
+ .setNameFormat("check-sys-mutex-count-%d").build());
+ Connection connForMutex = DriverManager.getConnection(getUrl(), props);
+ for (int i = 0; i < 5; i++) {
+ executorService.submit(new SystemMutexCaller(isSysMutexEmpty,
Review comment:
Makes sense, addressed. Thanks for the suggestion.
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
##########
@@ -269,4 +317,49 @@ public void
testInvalidConnUpdateCacheFrequencyShouldThrow() throws Exception {
}
}
}
+
+ /**
+ * Helper Runnable impl class that continuously keeps checking
+ * if SYSTEM.MUTEX contains any record until either interrupted or
+ * provided connection is closed
+ */
+ private static class SystemMutexCaller implements Runnable {
Review comment:
Nice, that sounds good way for sure. However, do you think keeping this
test as is (select queries) might be beneficial based on
1. Completeness of IT test
2. If by any chance we introduce checkAndPut on SYSTEM.MUTEX outside this
API maybe with some diff value or maybe by using column value in generating
rowkey, this specific test that focus on rowkey (tableName+schemaName as
non-null values) might not get impacted.
Althought 2) looks highly unlikely, maybe for completeness of IT, we can
keep this as is?
However, overriding `writeMutexCell()` sounds really nice one and less
complex. Let me explore tomorrow and get back, but somehow I feel completeness
with select query with specific where clause (as per your review comment,
updated select query with where clause to focus on specific row rather than
generic select query which would not work if this test is run in parallel with
other tests) might be better test.
Thanks
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
##########
@@ -104,10 +111,30 @@ public void testUpdateCacheForNeverUpdatedTable() throws
Exception {
String tableName = generateUniqueName();
String fullTableName = INDEX_DATA_SCHEMA +
QueryConstants.NAME_SEPARATOR + tableName;
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ AtomicBoolean isSysMutexEmpty = new AtomicBoolean(true);
+ ExecutorService executorService = Executors.newFixedThreadPool(5,
+ new ThreadFactoryBuilder().setDaemon(true)
+ .setNameFormat("check-sys-mutex-count-%d").build());
+ Connection connForMutex = DriverManager.getConnection(getUrl(), props);
+ for (int i = 0; i < 5; i++) {
+ executorService.submit(new SystemMutexCaller(isSysMutexEmpty,
+ connForMutex));
+ }
+ Thread.sleep(500);
Review comment:
Sort of warm up for threads to start executing select query :)
But yes, since we are not achieving anything major here, let me remove it if
you are strong about it?
----------------------------------------------------------------
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:
[email protected]
