Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2550: Switch to per-query exec rpc ......................................................................
Patch Set 3: (15 comments) http://gerrit.cloudera.org:8080/#/c/6535/3/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: PS3, Line 381: the total number : // of fragment instances > what's that? Done PS3, Line 386: i32 > Types.TFragmentIdx ? Done PS3, Line 412: 6: list<TPlanFragmentDestination> destinations > isn't that common across all instances? Done PS3, Line 414: fragment > instance? Done PS3, Line 435: coord_state_idx > not clear why the backend will care what index it was represented by in a c Done Line 441: 4: list<TPlanFragmentCtx> fragment_ctxs > are these in any particular order? i explained the ordering for fragment_instance_ctxs Line 453: // ReportExecStatus > other RPC names have a blank line after them, which makes them easier to fi Done PS3, Line 552: overall > what does "overall" mean here? this makes it seem like it's the merged sta changed code to match description PS3, Line 560: insert_exec_status > I guess this will hold the insert status for all instances, right? rephrased Line 563: 7: optional map<ErrorCodes.TErrorCode, TErrorLogEntry> error_log; > and same here. this will be the aggregated error log across all instances? rephrased PS3, Line 572: CancelPlanFragment > CancelQueryFInstances Done Line 669: > It would be helpful to add the RPC comment to show which RPC the following Done Line 699: > // PublishFilter Done PS3, Line 734: ReportExecStatus > Eventually this will report status for all the instances (at least in the p well, it's overall backend execution status (this could report an error even before any fragment instances are started), so that's why i thought 'exec status' is still the right scope Line 739: TCancelQueryFInstancesResult CancelQueryFInstances( i didn't want to name this CancelQuery, because that sounds too much like a client-related rpc -- To view, visit http://gerrit.cloudera.org:8080/6535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I20769e420711737b6b385c744cef4851cee3facd Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
