You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Sahil Takiar (Code Review)" <ge...@cloudera.org> on 2019/11/19 17:07:49 UTC
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14744
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Fixed a possible race condition where a query could transition from
RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/query_test/test_observability.py
7 files changed, 123 insertions(+), 71 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/1
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 9:
I'm not entirely sure why, but the original version of the test_error_query_state turned out to be flaky. Sometimes the profile would already be in an ERROR state before the client fetches any rows (perhaps that is expected behavior, not sure). I change it so that the query runs with a debug action and fails whenever the scan node calls GETNEXT, which should be more reliable.
I also made a bug fix to the tests. Originally the tests used the following asserts:
assert "Query State: RUNNING" in profile
assert "Impala Query State: RUNNING" in profile
The issue is that the asserts are redundant since 'Query State: RUNNING' is inside the string 'Impala Query State: RUNNING'. I fixed this using regexes to anchor the search string at the beginning of a line.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 15:34:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5327/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 02:43:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 10:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5243/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 10
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 23:29:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5303/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 20:13:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 07:12:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5114/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Nov 2019 04:21:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5065/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 1
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Nov 2019 17:53:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 9: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14744/9/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/14744/9/tests/common/impala_test_suite.py@245
PS9, Line 245:
nit: extra new line
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 22:19:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5235/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 03:12:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 2:
Re-based and added some more tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Nov 2019 19:00:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5180/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Dec 2019 22:20:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5305/
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 01:30:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 11:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5329/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 11
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 22:59:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14744
to look at the new patch set (#3).
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Previously, UpdateNonErrorOperationState would ignore attempts to make
illegal state transitions, now the method uses DCHECKs to ensure only
valid state transitions are attempted; this required fixing a possible race
condition where a query could transition from RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/query_test/test_observability.py
M tests/util/cancel_util.py
8 files changed, 190 insertions(+), 95 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/3
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Previously, UpdateNonErrorOperationState would ignore attempts to make
illegal state transitions, now the method uses DCHECKs to ensure only
valid state transitions are attempted; this required fixing a possible race
condition where a query could transition from RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Reviewed-on: http://gerrit.cloudera.org:8080/14744
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Sahil Takiar <st...@cloudera.com>
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/common/impala_test_suite.py
M tests/query_test/test_observability.py
8 files changed, 200 insertions(+), 99 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Sahil Takiar: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 12
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14744/9/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/14744/9/tests/common/impala_test_suite.py@245
PS9, Line 245: def get_impalad_cluster_size(cls):
> nit: extra new line
Done
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 10
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 22:59:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14744
to look at the new patch set (#10).
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Previously, UpdateNonErrorOperationState would ignore attempts to make
illegal state transitions, now the method uses DCHECKs to ensure only
valid state transitions are attempted; this required fixing a possible race
condition where a query could transition from RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/common/impala_test_suite.py
M tests/query_test/test_observability.py
8 files changed, 200 insertions(+), 99 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/10
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 10
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14744
to look at the new patch set (#5).
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Previously, UpdateNonErrorOperationState would ignore attempts to make
illegal state transitions, now the method uses DCHECKs to ensure only
valid state transitions are attempted; this required fixing a possible race
condition where a query could transition from RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/query_test/test_observability.py
7 files changed, 180 insertions(+), 95 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/5
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5068/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Nov 2019 19:45:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Dec 2019 22:24:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 11
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 15:53:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5189/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 7
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Dec 2019 03:26:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 20:13:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5305/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Dec 2019 22:24:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 11:
Carrying +2.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 11
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 15:53:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14744
to look at the new patch set (#2).
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Fixed a possible race condition where a query could transition from
RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/query_test/test_observability.py
M tests/util/cancel_util.py
8 files changed, 185 insertions(+), 95 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/2
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14744/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14744/2//COMMIT_MSG@27
PS2, Line 27: illegal state tran
> the (case->if) statement in UpdateNonErrorOperationState would ensure that
Good point. Updated the commit message.
wrt UpdateNonErrorOperationState, it seems the current behavior is to ignore state transitions that are invalid, I think it would be cleaner to enforce the transitions using DCHECKs and fix any attempts to perform an illegal state transition. this can help surface bugs in the code if the an illegal state transition is ever attempted.
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@a278
PS2, Line 278:
:
:
> i think the reason we chose to do this here is because we wanted to guarant
changed it so that if there is a async_exec_thread_ thread, it sets the state to PENDING before the async_exec_thread_ is created. this way when the Exec RPC returns, the state should always be PENDING.
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@780
PS2, Line 780: result_metadata_ = metadata_op_result.schema
> I think with a metadata op it is expected to transition directly to FINISHE
Yeah, I made this change because thats what all the other operations do, even the ones that don't run asynchronously. So I think it makes sense to keep this consistent with the rest of the operations. Plus, it makes the state machine easier to reason about if everything transitions from INITIALIZED -> [PENDING] -> RUNNING -> FINISHED.
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@929
PS2, Line 929: $0 -> $1";
> nit: how about:
Done
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Nov 2019 03:37:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14744/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/14744/3/be/src/service/client-request-state.cc@506
PS3, Line 506: UpdateNonErrorExecState(ExecState::PENDING);
> This will actually mark the query pending even if the async-exec-thread cre
Yeah, if the thread creation fails the state will be PENDING, and will eventually be set to ERROR by ClientRequestState::Cancel(status).
So prior to this patch, if thread creation fails, the state would go from INITIALIZED to ERROR.
After this patch, if thread creation fails, the state would go from INITIALIZED to PENDING to ERROR.
Agree, don't think that change should be a problem.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Sun, 24 Nov 2019 22:45:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14744/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14744/2//COMMIT_MSG@27
PS2, Line 27: RUNNING to PENDING
the (case->if) statement in UpdateNonErrorOperationState would ensure that it does not transition back to a previous state. This was added specifically because of the decision to transition to PENDING state in the Exec RPC thread.
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@a278
PS2, Line 278:
:
:
i think the reason we chose to do this here is because we wanted to guarantee that the state transitions to the next state (PENDING or RUNNING) when the client's Exec RPC returns. I am not sure if there are any tests that rely on this assumption (if there are then your change can make them flaky)
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@780
PS2, Line 780: UpdateNonErrorExecState(ExecState::RUNNING);
I think with a metadata op it is expected to transition directly to FINISHED since there is actually nothing executing at this point. Also if we set this to running here, the caller of this ImpalaServer::ExecuteMetadataOp immediately transitions it to a terminal state (FINISHED or ERROR). Dont feel strongly about this either way, so feel free to ignore.
http://gerrit.cloudera.org:8080/#/c/14744/2/be/src/service/client-request-state.cc@929
PS2, Line 929: Unexpected state
nit: how about:
"Illegal state transition: " << ExecStateToString(old_state) << " -> " << ExecStateToString(new_state)
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Nov 2019 22:00:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 5: Code-Review+2
Carrying +2
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Dec 2019 22:24:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Thomas Tauber-Marshall, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14744
to look at the new patch set (#9).
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Previously, UpdateNonErrorOperationState would ignore attempts to make
illegal state transitions, now the method uses DCHECKs to ensure only
valid state transitions are attempted; this required fixing a possible race
condition where a query could transition from RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/common/impala_test_suite.py
M tests/query_test/test_observability.py
8 files changed, 200 insertions(+), 98 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/9
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 9
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 5:
Removed the new assertions I added to cancel_util.py. Turns out, when a query is cancelled it can remain in the FINISHED state (IMPALA-1262). Addressing this is beyond the scope of this JIRA, and the assertions should be re-added when we tackle IMPALA-1262.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Dec 2019 21:52:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Sahil Takiar (Code Review)" <ge...@cloudera.org>.
Sahil Takiar has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
IMPALA-6894: Use an internal representation of query states in ClientRequestState
Re-factors the state machine of ClientRequestState so that it uses an
internal state represetation rather than using the one defined by
TOperationState. The possible states are defined in
ClientRequestState::ExecState and the possible state transitions are
outlined in client-request-state.h and enforced in
ClientRequestState::UpdateNonErrorExecState. The states defined in
ClientRequestState::ExecState are the same states currently used in
TOperationState. This patch simply makes it easy to define new states
in the future.
The value of ClientRequestState::ExecState is exposed to clients via the
entry "Impala Query State" in the runtime profile. It is meant to be the
Impala specific version of "Query State" (which is the Beeswax state).
This allows Impala to expose its internal state without breaking existing
clients that might rely on the value of "Query State".
Additional Bug Fixes:
* Previously, UpdateNonErrorOperationState would ignore attempts to make
illegal state transitions, now the method uses DCHECKs to ensure only
valid state transitions are attempted; this required fixing a possible race
condition where a query could transition from RUNNING to PENDING
* The ClientRequestState state is now tracked using an AtomicEnum, which
fixes a few possible race conditions where the state was being read
without holding the ClientRequestState::lock_
Testing:
* Ran core tests
* Added test to make sure "Impala Query State" is populated
Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/service/impala-http-handler.cc
M be/src/service/impala-server.cc
M tests/query_test/test_observability.py
7 files changed, 180 insertions(+), 95 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/14744/7
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 7
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 4: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5303/
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 4
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 00:54:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14744/3/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/14744/3/be/src/service/client-request-state.cc@506
PS3, Line 506: UpdateNonErrorExecState(ExecState::PENDING);
This will actually mark the query pending even if the async-exec-thread creation failed. I dont think this should be a problem though because there is no condition specified on that state that assumes otherwise.
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 3
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Nov 2019 19:21:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 )
Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState
......................................................................
Patch Set 11: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630
Gerrit-Change-Number: 14744
Gerrit-PatchSet: 11
Gerrit-Owner: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <mi...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 03:27:02 +0000
Gerrit-HasComments: No