Dan Hecht has posted comments on this change. Change subject: IMPALA-561: Allow multiple callbacks in a thread resource pool. ......................................................................
Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/2430/4/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: Line 699: thread_callback_id_ how about 'thread_avail_cb_id_' to be more specific? http://gerrit.cloudera.org:8080/#/c/2430/4/be/src/runtime/thread-resource-mgr.cc File be/src/runtime/thread-resource-mgr.cc: Line 52: thread_callbacks_.clear(); would it make sense to DCHECK that all callbacks were removed? Line 95: // until wrap around occurs. this comment doesn't seem to match the code given that size() can't wrap. Line 108: thread_callbacks_[id] = NULL; does this leave the vector sparse under certain patterns? or is it always Add Add Add Remove Remove Remove? Line 121: num_callbacks_ why do we use this instead of just doing thread_callbacks_.size()? -- To view, visit http://gerrit.cloudera.org:8080/2430 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iddfff1feef0b59d407994ad3bc560166acbfa623 Gerrit-PatchSet: 4 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
