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

Reply via email to