Joe McDonnell has posted comments on this change. Change subject: IMPALA-5488: Fix handling of exclusive HDFS file handles ......................................................................
Patch Set 2: (7 comments) I will file a JIRA for the exclusive vs cached design issue. http://gerrit.cloudera.org:8080/#/c/7181/2/be/src/runtime/disk-io-mgr-scan-range.cc File be/src/runtime/disk-io-mgr-scan-range.cc: PS2, Line 292: This will not use a cached handle. > but it's still in the cache after this call, right? it must be given L301, Done PS2, Line 304: stringstream ss; : ss << > Substitute() might be cleaner, but okay to ignore. Done http://gerrit.cloudera.org:8080/#/c/7181/2/be/src/runtime/disk-io-mgr.cc File be/src/runtime/disk-io-mgr.cc: PS2, Line 1214: fh > nit: fh == nullptr, per our style (no implicit conversion to bool). Done Line 1215: if (cache_hit) { > maybe DCHECK(!require_new)? Done PS2, Line 1238: dummy > maybe rename that to cache_hit and then DCHECK(!cache_hit); Done http://gerrit.cloudera.org:8080/#/c/7181/2/be/src/runtime/disk-io-mgr.h File be/src/runtime/disk-io-mgr.h: PS2, Line 777: ` > nit: use ' rather than ` Done PS2, Line 786: ` > nit: use ' rather than ` Done -- To view, visit http://gerrit.cloudera.org:8080/7181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c03696984285cc9ce463edd969c5149cd83a861 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
