Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8414 )
Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation ...................................................................... Patch Set 12: Code-Review+2 (1 comment) Please see if Tianyi has any more comments too. http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h@93 PS9, Line 93: void Cancel(); > Let me take a look through the current patchset first. Even the other methods seem to operate entirely (or almost entirely?) on the 'reader' state, rather than the diskiomgr 'this' itself. Even the lock is the reader lock. So it seems they are more operations on the context. I think the current change is okay as is, but i think we could consider moving even more things into here in a purely mechanical change in the future. -- To view, visit http://gerrit.cloudera.org:8080/8414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 Gerrit-Change-Number: 8414 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 21 Nov 2017 04:27:40 +0000 Gerrit-HasComments: Yes