madrob commented on a change in pull request #198:
URL: https://github.com/apache/solr/pull/198#discussion_r665578729
##########
File path:
solr/core/src/test/org/apache/solr/handler/admin/ThreadDumpHandlerTest.java
##########
@@ -95,12 +99,23 @@ public void doTestMonitor(final boolean
checkBlockedThreadViaPolling) throws Exc
failures.add("never saw lockIsHeldLatch released");
return;
}
- assertQ(req("qt", "/admin/threads", "indent", "true")
- // monitor owner 'ownerT'
- // (which *MAY* also be waiting on doneWithTestLatch, but may
not have reached that line yet)
- ,
"//lst[@name='thread'][str[@name='name'][.='test-thread-monitor-owner']]"
- + "
[arr[@name='monitors-locked']/str[contains(.,'TestMonitorStruct')]]"
- );
+
+ @SuppressWarnings({"unchecked"})
+ NamedList<Object> threads = (NamedList<Object>)
readProperties()._get("system/threadDump", null);
+ boolean found = false;
+ for (Map.Entry<String, Object> threadEntry : threads) {
+ @SuppressWarnings({"unchecked"})
+ NamedList<Object> thread = (NamedList<Object>) threadEntry.getValue();
+ // monitor owner 'ownerT'
+ // (which *MAY* also be waiting on doneWithTestLatch, but may not have
reached that line yet)
+ if (thread._getStr("name",
null).contains("test-thread-monitor-owner")) {
+ if (thread._get("monitors-locked",
"").toString().contains("TestMonitorStruct")) {
+ found = true;
+ break;
+ }
+ }
+ }
+ assertTrue(found);
Review comment:
Please add a failure message to this assertion.
##########
File path:
solr/core/src/test/org/apache/solr/handler/admin/ThreadDumpHandlerTest.java
##########
@@ -214,16 +261,36 @@ public void doTestOwnableSync(final boolean
checkWaitingThreadViaPolling) throws
Thread.sleep(10); // 10ms at a time, at most 5 sec total
}
if (lock.hasQueuedThread(blockedT)) {
- assertQ(req("qt", "/admin/threads", "indent", "true")
- // same lock owner 'ownerT'
- ,
"//lst[@name='thread'][str[@name='name'][.='test-thread-sync-lock-owner']]"
- + "
[arr[@name='synchronizers-locked']/str[contains(.,'ReentrantLock')]]"
- // blocked thread 'blockedT', waiting on the lock
- ,
"//lst[@name='thread'][str[@name='name'][.='test-thread-sync-lock-blocked']]"
- + " [str[@name='state'][.='WAITING']]"
- + "
[lst[@name='lock-waiting'][lst[@name='owner']/str[.='test-thread-sync-lock-owner']]]"
- );
-
+ @SuppressWarnings({"unchecked"})
+ NamedList<Object> blockedThreads = (NamedList<Object>)
readProperties()._get("system/threadDump", null);
+ found = false;
+ for (Map.Entry<String, Object> threadEntry : blockedThreads) {
+ @SuppressWarnings({"unchecked"})
+ NamedList<Object> thread = (NamedList<Object>)
threadEntry.getValue();
+ // lock owner 'ownerT'
+ if (thread._getStr("name",
null).contains("test-thread-sync-lock-owner")) {
+ if (thread._get("synchronizers-locked",
"").toString().contains("ReentrantLock")) {
+ found = true;
+ break;
+ }
+ }
+ }
Review comment:
Can this logic be refactored into a helper method? It looks like it's
been repeated several times already, looking for a thread with a given name and
some other condition that could be expressed via a Predicate.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
