You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Ho (Code Review)" <ge...@cloudera.org> on 2016/09/29 01:02:41 UTC
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Michael Ho has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4558
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
HdfsScanNodeBase::Close() may add its outstanding DiskIO context to
RuntimeState::reader_contexts_ to be unregistered later when the
fragment is closed. In a plan fragment with multiple HDFS scan nodes,
it's possible for HdfsScanNodeBase::Close() to be called concurrently.
To allow safe concurrent accesses, this change adds a SpinLock to
synchronize accesses to 'reader_contexts_' in RuntimeState.
Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
A testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test
M tests/query_test/test_queries.py
6 files changed, 46 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/4558/1
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
Line 167: /// TODO: Attach the reader context to the last row batch instead.
> This TODO seems too speculative to me, unclear whether the proposed approac
Works for me. If the eventual direction is not clear, then let's remove the TODO.
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4558/3/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:
Line 166: select * from t t1 left join t t2 on t1.x > 0
> did you verify that this query reproduces a crash?
Confirmed by Taras. Thanks Taras !
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:
Line 304: }
> reader_contexts_.clear()
Done
http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
Line 167: void DeferIOContextUnregistration(DiskIoRequestContext* reader_context);
> AcquireReaderContext()?
Done
Line 170: void UnregisterIOContexts();
> UnregisterReaderContexts()?
Done
http://gerrit.cloudera.org:8080/#/c/4558/1/testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test:
Line 4: # whose children are scan nodes. When scan nodes are closed concurrently, make
> I don't think the last sentence is necessary. It's likely to become stale p
Rephrased the comment in the new patch.
http://gerrit.cloudera.org:8080/#/c/4558/1/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
Line 131: def test_single_node_topn(self, vector):
> This new test seems kind of misleading. The cause of this are two scan node
The new test is moved to single-node-nlj.test in the new patch. Rewrote the query to be NLJ which also exercises the same path.
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 3:
> (1 comment)
Taras, were you also able to verify that the fix solves the crash?
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:
Line 302: for (DiskIoRequestContext* context: reader_contexts_) {
Nit clang-format puts a space before :, should do that or consistency.
http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
Line 167: /// TODO: Attach the reader context to the last row batch instead.
This TODO seems too speculative to me, unclear whether the proposed approach is the right one, since it's usually simpler not to attach things to row batches, and the current approach seems fine to me.
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 3: Code-Review+2
Change looks good to me as long as the test case also reproduces the crash.
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:
Line 304: }
reader_contexts_.clear()
http://gerrit.cloudera.org:8080/#/c/4558/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
Line 167: void DeferIOContextUnregistration(DiskIoRequestContext* reader_context);
AcquireReaderContext()?
I think the detail that these can only be reader contexts is important. "IOContext" seems more general, e.g. read/write contexts are not interesting here.
The "Acquire" terminology is more similar to what we use elsewhere. We are transferring ownership of the context to the runtime state because the context lives until the end of the fragment.
Might be good to add a brief TODO to indicate what the eventual solution should look like, e.g., attach the resources to the final row batch.
Line 170: void UnregisterIOContexts();
UnregisterReaderContexts()?
http://gerrit.cloudera.org:8080/#/c/4558/1/testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-topn.test:
Line 4: # whose children are scan nodes. When scan nodes are closed concurrently, make
I don't think the last sentence is necessary. It's likely to become stale pretty soon (at least I hope so).
http://gerrit.cloudera.org:8080/#/c/4558/1/tests/query_test/test_queries.py
File tests/query_test/test_queries.py:
Line 131: def test_single_node_topn(self, vector):
This new test seems kind of misleading. The cause of this are two scan nodes within the same fragment, the top-n nodes just happen to trigger the concurrent Close() pattern. This bug seems more in line with IMPALA-561 where you added a test to single-node-nlj.test.
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
HdfsScanNodeBase::Close() may add its outstanding DiskIO context to
RuntimeState::reader_contexts_ to be unregistered later when the
fragment is closed. In a plan fragment with multiple HDFS scan nodes,
it's possible for HdfsScanNodeBase::Close() to be called concurrently.
To allow safe concurrent accesses, this change adds a SpinLock to
synchronize accesses to 'reader_contexts_' in RuntimeState.
Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Reviewed-on: http://gerrit.cloudera.org:8080/4558
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
5 files changed, 39 insertions(+), 7 deletions(-)
Approvals:
Internal Jenkins: Verified
Dan Hecht: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.cc
File be/src/runtime/runtime-state.cc:
Line 302: for (DiskIoRequestContext* context: reader_contexts_) {
> Nit clang-format puts a space before :, should do that or consistency.
Done
http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
Line 165: /// Takes ownership of a scan node's reader context and unregisters it when the
> "Takes ownership of the given reader context which may still hold used IO b
It's "automatic" from the perspective of the caller but I know what you mean.
Line 167: /// TODO: Attach the reader context to the last row batch instead.
> Works for me. If the eventual direction is not clear, then let's remove the
Sure. Let's remove it for now but that sounds like an attractive option if unregistration happens at the end of the lifetime of the IO buffer.
http://gerrit.cloudera.org:8080/#/c/4558/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:
Line 215: ---- QUERY
> move this below the test for IMPALA-561 to cluster related tests together
Done
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 3:
Taras also verified the fix works with the same query.
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4558/2/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:
Line 165: /// Takes ownership of a scan node's reader context and unregisters it when the
"Takes ownership of the given reader context which may still hold used IO buffers."
The comment makes the unregistration sounds "automatic", but you actually need to call UnregisterReaderContexts(). Please rephrase slightly.
http://gerrit.cloudera.org:8080/#/c/4558/2/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:
Line 215: ---- QUERY
move this below the test for IMPALA-561 to cluster related tests together
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has uploaded a new patch set (#3).
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
HdfsScanNodeBase::Close() may add its outstanding DiskIO context to
RuntimeState::reader_contexts_ to be unregistered later when the
fragment is closed. In a plan fragment with multiple HDFS scan nodes,
it's possible for HdfsScanNodeBase::Close() to be called concurrently.
To allow safe concurrent accesses, this change adds a SpinLock to
synchronize accesses to 'reader_contexts_' in RuntimeState.
Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
5 files changed, 39 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/4558/3
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4558/3/testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
File testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test:
Line 166: select * from t t1 left join t t2 on t1.x > 0
did you verify that this query reproduces a crash?
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has uploaded a new patch set (#2).
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
HdfsScanNodeBase::Close() may add its outstanding DiskIO context to
RuntimeState::reader_contexts_ to be unregistered later when the
fragment is closed. In a plan fragment with multiple HDFS scan nodes,
it's possible for HdfsScanNodeBase::Close() to be called concurrently.
To allow safe concurrent accesses, this change adds a SpinLock to
synchronize accesses to 'reader_contexts_' in RuntimeState.
Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
---
M be/src/exec/hdfs-scan-node-base.cc
M be/src/runtime/plan-fragment-executor.cc
M be/src/runtime/runtime-state.cc
M be/src/runtime/runtime-state.h
M testdata/workloads/functional-query/queries/QueryTest/single-node-nlj.test
5 files changed, 38 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/4558/2
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
[Impala-ASF-CR] IMPALA-4180: Synchronize accesses to RuntimeState::reader contexts
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4180: Synchronize accesses to RuntimeState::reader_contexts_
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4558
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I911fda526a99514b12f88a3e9fb5952ea4fe1973
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No