[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 19:09:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. IMPALA-7055: fix race with DML errors Error statuses could be lost because backend_exec_complete_barrier_ went to 0 before the query was transitioned to an error state. Reordering the UpdateExecState() and backend_exec_complete_barrier_ calls prevents this race. Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Reviewed-on: http://gerrit.cloudera.org:8080/10491 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h 2 files changed, 19 insertions(+), 8 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: It looks like that flake occurred on a different branch too. I'll try to merge this again since it fixes a test failure. -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 15:43:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2547/ -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 15:43:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: Hit IMPALA-7067. Will hold off on merging in case this patch increased the changes of hitting that flakiness. -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 04:59:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2540/ -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 04:20:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2540/ -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 00:52:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 00:52:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 00:30:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 1: (2 comments) Maybe take another look to make sure that my change was what you intended http://gerrit.cloudera.org:8080/#/c/10491/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10491/1//COMMIT_MSG@12 PS1, Line 12: Updating backend_exec_complete_barrier_ isn't actually necessary : when handling error > BTW, that's kind of true, except that if we've already seen an error then w Yeah you're right, it is necessary if the state was already RETURNED_RESULTS and we got an error after that transition. http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc@688 PS1, Line 688: if (VLOG_QUERY_IS_ON && pending_backends >= 0) { : VLOG_QUERY << "Backend completed:" : << " host=" << TNetworkAddressToString(backend_state->impalad_address()) : << " remaining=" << pending_backends : << " query_id=" << PrintId(query_id()); : BackendState::LogFirstInProgress(backend_states_); : } > I think I messed this up because I wanted this log to print before the logg Yeah that makes sense actually, put it back up the top. -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 00:28:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Hello Bikramjeet Vig, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10491 to look at the new patch set (#2). Change subject: IMPALA-7055: fix race with DML errors .. IMPALA-7055: fix race with DML errors Error statuses could be lost because backend_exec_complete_barrier_ went to 0 before the query was transitioned to an error state. Reordering the UpdateExecState() and backend_exec_complete_barrier_ calls prevents this race. Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h 2 files changed, 19 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/10491/2 -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10491/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10491/1//COMMIT_MSG@12 PS1, Line 12: Updating backend_exec_complete_barrier_ isn't actually necessary : when handling error BTW, that's kind of true, except that if we've already seen an error then we won't transition. So, handling an error here doesn't guarantee the notify with happen. -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 00:04:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 1: Code-Review+2 (1 comment) Thanks for taking care of that Tim. It looks right. http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/10491/1/be/src/runtime/coordinator.cc@688 PS1, Line 688: if (VLOG_QUERY_IS_ON && pending_backends >= 0) { : VLOG_QUERY << "Backend completed:" : << " host=" << TNetworkAddressToString(backend_state->impalad_address()) : << " remaining=" << pending_backends : << " query_id=" << PrintId(query_id()); : BackendState::LogFirstInProgress(backend_states_); : } I think I messed this up because I wanted this log to print before the logging in UpdateExecState() (so it'd be clearer where the error comes from we are updating the exec state). Maybe the order isn't actually helpful, but if you think it does then an option is to use backend_exec_complete_barrier_->pending() to get pending_backends earlier without notifying until later. -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 24 May 2018 00:02:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10491 Change subject: IMPALA-7055: fix race with DML errors .. IMPALA-7055: fix race with DML errors Error statuses could be lost because backend_exec_complete_barrier_ went to 0 before the query was transitioned to an error state. Updating backend_exec_complete_barrier_ isn't actually necessary when handling error - UpdateExecState() already signals the barrier when transitioning to an ERROR/CANCELLED terminal state. Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h 2 files changed, 15 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/10491/1 -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7055: fix race with DML errors
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10491 ) Change subject: IMPALA-7055: fix race with DML errors .. Patch Set 1: I'm still in the process of testing this but wanted to get feedback on whether it's the correct fix. -- To view, visit http://gerrit.cloudera.org:8080/10491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idafd0b342e77a065be7cc28fa8c8a9df445622c2 Gerrit-Change-Number: 10491 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 23 May 2018 22:14:00 + Gerrit-HasComments: No