Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/13522 )
Change subject: IMPALA-8561: Eliminate mtime=-1 for HDFS scan ranges (part 1) ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/handle-cache.inline.h File be/src/runtime/io/handle-cache.inline.h: http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/handle-cache.inline.h@93 PS4, Line 93: DCHECK_GT(mtime, 0); > super minor nit: is 0 invalid ? I was thinking about this, and I decided that I would want to know if it is 0. Modification time is defined to be UNIX time, so a value of 0 indicates something unexpected with a chance that it is always 0. http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/request-ranges.h File be/src/runtime/io/request-ranges.h: http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/request-ranges.h@170 PS4, Line 170: 'mtime' matches : // the modified time of the cached file in the HDFS cache > stale Good point, fixed. http://gerrit.cloudera.org:8080/#/c/13522/4/be/src/runtime/io/request-ranges.h@244 PS4, Line 244: int64_t mtime, > Nice to have one comment about it. Added comments for mtime and is_erasure_coded. -- To view, visit http://gerrit.cloudera.org:8080/13522 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I48b7ed60d6ab9104b993237b4fe23de5dc058672 Gerrit-Change-Number: 13522 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 11 Jun 2019 17:09:40 +0000 Gerrit-HasComments: Yes
