Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8408 )
Change subject: IMPALA-6121: remove I/O mgr request context cache ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-reader-context.h File be/src/runtime/disk-io-mgr-reader-context.h: http://gerrit.cloudera.org:8080/#/c/8408/4/be/src/runtime/disk-io-mgr-reader-context.h@319 PS4, Line 319: if (!is_on_queue_ && num_threads_in_op_.Load() == 0 && !done_) { > I'd have to think about this carefully to understand if the Acquire barrier Just following up on this. It turns out that the use of Load() here and the order of checks *was* significant. If I changed, disk-io-mgr-stress-test crashed after ~300 iterations. Specifically, is_on_queue_ needs to be read before num_threads_in_op_ to avoid a race with IncrementRequestThreadAndQueue(). https://gerrit.cloudera.org/#/c/8414/ cleans this up and documents it better. -- To view, visit http://gerrit.cloudera.org:8080/8408 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91414eceaa4938fccd74686fe6bebede6ef36108 Gerrit-Change-Number: 8408 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 16 Nov 2017 19:06:54 +0000 Gerrit-HasComments: Yes
