Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10245 )

Change subject: IMPALA-6953: clean up DiskIoMgr
......................................................................


Patch Set 7:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr-internal.h
File be/src/runtime/io/disk-io-mgr-internal.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr-internal.h@79
PS7, Line 79: /
> pre-existing nit: should only be //
Done


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr-internal.h@98
PS7, Line 98:
> extra space
Done


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr.h
File be/src/runtime/io/disk-io-mgr.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/disk-io-mgr.h@395
PS7, Line 395: HandleWriteFinished
> ()
Done


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h
File be/src/runtime/io/request-context.h:

http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h@196
PS7, Line 196: gets
> get
Done


http://gerrit.cloudera.org:8080/#/c/10245/7/be/src/runtime/io/request-context.h@498
PS7, Line 498: inline void RequestContext::IncrementDiskThreadAfterDequeue(int 
disk_id) {
> I think this wrapper didn't exist in the old code.  Is the PerDiskState com
disk_states_ is now private, so not accessible outside RequestContext. I can't 
remember exactly why I had the forward declaration in protected, just checking 
now... ok, so I moved it to private and it compiles, so I'll go with that.



--
To view, visit http://gerrit.cloudera.org:8080/10245
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a6e393f8c01d10143cbac91108af37f6498c1b1
Gerrit-Change-Number: 10245
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Tue, 22 May 2018 18:45:24 +0000
Gerrit-HasComments: Yes

Reply via email to