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 2022/11/16 09:13:58 UTC

[GitHub] [druid] rohangarg opened a new pull request, #13373: Improve performance for ReadableInputStreamFrameChannel

rohangarg opened a new pull request, #13373:
URL: https://github.com/apache/druid/pull/13373

   Moves away from sleep wait in ReadableInputStreamFrameChannel to wait/notify model.
   This improves the performance for DurableStorageOutputChannel by around 10x on local storage backed durable output channel.
   
   This PR has:
   
   - [x] been self-reviewed.
      - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
   - [ ] added documentation for new or modified features or behaviors.
   - [ ] a release note entry in the PR description.
   - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
   - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
   - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [ ] added integration tests.
   - [ ] been tested in a test Druid cluster.
   


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [druid] cryptoe merged pull request #13373: Improve performance for ReadableInputStreamFrameChannel

Posted by GitBox <gi...@apache.org>.
cryptoe merged PR #13373:
URL: https://github.com/apache/druid/pull/13373


-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [druid] gianm commented on a diff in pull request #13373: Improve performance for ReadableInputStreamFrameChannel

Posted by GitBox <gi...@apache.org>.
gianm commented on code in PR #13373:
URL: https://github.com/apache/druid/pull/13373#discussion_r1024311128


##########
processing/src/main/java/org/apache/druid/frame/channel/ReadableInputStreamFrameChannel.java:
##########
@@ -186,7 +190,15 @@ private void startReading()
                 totalInputStreamBytesRead += bytesRead;
                 if (backpressureFuture != null) {
                   keepReading = false;
-                  backpressureFuture.addListener(() -> keepReading = true, Execs.directExecutor());
+                  backpressureFuture.addListener(
+                      () -> {
+                        keepReading = true;
+                        synchronized (readMonitor) {

Review Comment:
   There's a potential race here where:
   
   1) The reader thread sees `!keepReading` on line 153
   2) `keepReading` is set to `true` here
   3) readMonitor is notified here
   4) The reader thread enters the `synchronized (readMonitor)` block on line 157, and waits for no reason
   
   We can fix it by setting `keepReading` inside this block, and double-checking it in the reader thread on entry into the `synchronized (readMonitor)` block.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [druid] rohangarg commented on a diff in pull request #13373: Improve performance for ReadableInputStreamFrameChannel

Posted by GitBox <gi...@apache.org>.
rohangarg commented on code in PR #13373:
URL: https://github.com/apache/druid/pull/13373#discussion_r1025442237


##########
processing/src/main/java/org/apache/druid/frame/channel/ReadableInputStreamFrameChannel.java:
##########
@@ -186,7 +190,15 @@ private void startReading()
                 totalInputStreamBytesRead += bytesRead;
                 if (backpressureFuture != null) {
                   keepReading = false;
-                  backpressureFuture.addListener(() -> keepReading = true, Execs.directExecutor());
+                  backpressureFuture.addListener(
+                      () -> {
+                        keepReading = true;
+                        synchronized (readMonitor) {

Review Comment:
   Thanks for catching it Gian! Fixed 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.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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