You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/01/29 15:20:40 UTC

[GitHub] [druid] leventov opened a new issue #9285: Dangling CountDownLatch.await() in tests

leventov opened a new issue #9285: Dangling CountDownLatch.await() in tests
URL: https://github.com/apache/druid/issues/9285
 
 
   There are several places in tests where the boolean result of `CountDownLatch.await(timeout)` is not tested. It should properly be wrapped into `assertTrue()` in (almost) all cases. Currently, these tests don't really verify the correctness of the production code.
   
   See https://github.com/code-review-checklists/java-concurrency#check-await
   
   To systematically eradicate these bugs, either the configuration for "Result of method call is ignored" inspection should be edited and the inspection is enabled on error level (which I think is preferable because we want to strictly check this inspection anyway), or a separate [Structural Search pattern](https://github.com/apache/druid/blob/master/dev/teamcity.md#creating-a-custom-inspection-from-a-structural-search-pattern) could be easily created.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] leventov commented on issue #9285: Dangling CountDownLatch.await() in tests

Posted by GitBox <gi...@apache.org>.
leventov commented on issue #9285: Dangling CountDownLatch.await() in tests
URL: https://github.com/apache/druid/issues/9285#issuecomment-594364556
 
 
   `$x$.awaitTermination($y$, $z$);` structural search

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] leventov commented on issue #9285: Dangling CountDownLatch.await() in tests

Posted by GitBox <gi...@apache.org>.
leventov commented on issue #9285: Dangling CountDownLatch.await() in tests
URL: https://github.com/apache/druid/issues/9285#issuecomment-593066477
 
 
   Other methods:
    - `ExecutorService.awaitTermination`, e. g. in [`IntermediaryDataManager`](https://github.com/apache/druid/blob/master/indexing-service/src/main/java/org/apache/druid/indexing/worker/IntermediaryDataManager.java#L167) should probably at least log something if it fails to terminate in the specified time, FYI @jihoonson. The Structural search pattern should be marked as "within type hierarchy".
    - `AsynchronousChannelGroup.awaitTermination`
    - `ForkJoinPool.awaitQuiescence`
    - `Semaphore.tryAcquire`
    - `Condition.await`, "within type hierarchy"
    - `Lock.tryLock`, "within type hierarchy"
    - `Process.waitFor`
    - `Monitor.enter` (Guava)
    - `RateLimiter.tryAcquire` (Guava)
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] akashvyom commented on issue #9285: Dangling CountDownLatch.await() in tests

Posted by GitBox <gi...@apache.org>.
akashvyom commented on issue #9285: Dangling CountDownLatch.await() in tests
URL: https://github.com/apache/druid/issues/9285#issuecomment-601022637
 
 
   Hi,
   I would like to work on this ticket, I will spend some time getting myself familiar with the codebase and try to track down where to fix this issue.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org

[GitHub] [druid] jihoonson commented on issue #9285: Dangling CountDownLatch.await() in tests

Posted by GitBox <gi...@apache.org>.
jihoonson commented on issue #9285: Dangling CountDownLatch.await() in tests
URL: https://github.com/apache/druid/issues/9285#issuecomment-594238100
 
 
   @leventov thank you for finding it. Is there a way to forbid it with a code style or an inspection rule?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org