Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9447 )
Change subject: IMPALA-6512: Maintenace thread period should respect FLAGS_datastream_sender_timeout_ms ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/9447/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9447/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-6512: Maintenace thread period should respect FLAGS_datastream_sender_timeout_ms is that the right JIRA? Wasn't that one addressed with Lars' error message change? http://gerrit.cloudera.org:8080/#/c/9447/1/be/src/runtime/krpc-data-stream-mgr.cc File be/src/runtime/krpc-data-stream-mgr.cc: http://gerrit.cloudera.org:8080/#/c/9447/1/be/src/runtime/krpc-data-stream-mgr.cc@367 PS1, Line 367: DCHECK_GE(STREAM_EXPIRATION_TIME_MS, 10000); what does this mean? what's the reason for not using STREAM_EXPIRATION_TIME_MS directly below? http://gerrit.cloudera.org:8080/#/c/9447/1/be/src/runtime/krpc-data-stream-mgr.cc@369 PS1, Line 369: (1 is 1ms a good lower bound, or should we make it 10? Though, I guess it only matters when --datastream_sender_timeout_ms is set to something really really low. http://gerrit.cloudera.org:8080/#/c/9447/1/be/src/runtime/krpc-data-stream-mgr.cc@375 PS1, Line 375: int64_t now = MonotonicMillis(); maybe hoist that up, and use the same 'now' value for the entire iteration. -- To view, visit http://gerrit.cloudera.org:8080/9447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I804cef7cc991007ec44375f8eac804aa2df46bd7 Gerrit-Change-Number: 9447 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Comment-Date: Mon, 26 Feb 2018 18:45:39 +0000 Gerrit-HasComments: Yes
