[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Dan Hecht has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: > Yes I will try tomorrow. Dan could you say a little about why you > do not think so? the hdfs scan node path that crashes doesn't use this accessor. I think the error log logic is probably broken in multiple ways and we don't understand all the ways it's broken. We need to repro in order to be confident we're fixing, and to regression test. To repro, you could try adding a stress option that injects synthetic errors into queries. -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: ok I know. I will do that part later. -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: I will update how the crash can happen in commit message and in Jira later tonight. -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: Yes I will try tomorrow. Dan could you say a little about why you do not think so? -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/2829/1/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: Line 221: lock_guard l(error_log_lock_); > I'm not sure if this works, the lock is released once out of scope. You mig I did not test this. but i think it should work. I believe this will be out of scope once return is completed. I will check tomorrow. thanks for pointing out -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/2829/1/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: Line 221: lock_guard l(error_log_lock_); I'm not sure if this works, the lock is released once out of scope. You might want to check that. -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Dan Hecht has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: I don't think this fixes the hdfs-scan-node race. can you try reproducing the issue? -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Huaisi Xu has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/2829/1/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: Line 196: error_log > Shouldn't you update all the direct references of error_log_ to use error_l I think if that in the same class I do not have to, right? There are runtime states kept by coordinator, which is not related to this one... I am not sure whether you are referring to those. -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Huaisi Xu Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-3385: hold locks when returning error_log .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/2829/1/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: Line 196: error_log Shouldn't you update all the direct references of error_log_ to use error_log()? Here and elsewhere. -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu Gerrit-Reviewer: Bharath Vissapragada Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3385: hold locks when returning error_log
Huaisi Xu has uploaded a new change for review. http://gerrit.cloudera.org:8080/2829 Change subject: IMPALA-3385: hold locks when returning error_log .. IMPALA-3385: hold locks when returning error_log I will update commit message and the Jira with detail later. Does this fix look good? Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc --- M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h 2 files changed, 6 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/29/2829/1 -- To view, visit http://gerrit.cloudera.org:8080/2829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3a7e3d22e26147ada780aae5aed1f2e25a515afc Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu
