You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Yongzhi Chen (Code Review)" <ge...@cloudera.org> on 2019/01/29 18:50:58 UTC

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen 

Yongzhi Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12296


Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 16 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/1
-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 7:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/1965/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 23:10:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

The change makes profiles prefix counters consistent.
Only TimeSeriesCounters are affected.

Testing:
Add TimeSeriesCounter prefix checks in test_observability tests.
Manual Tests: Run query and check profile for MemoryUsage and
ThreadUsage. Following section of profile shows that TimeSeriesCounters
are consistent with other counters:

    Fragment F00:
        . . .
        Fragment Instance Lifecycle Event Timeline: 273.841ms
           - Prepare Finished: 1.511ms (1.511ms)
           . . .
         - MemoryUsage(500.000ms): 2.81 MB
         - ThreadUsage(500.000ms): 1
         - AverageThreadTokens: 1.00
         - BloomFilterBytes: 1.00 MB (1048576)
         - ExchangeScanRatio: 0.00
         - PeakMemoryUsage: 54.26 MB (56891933)
         - PeakReservation: 53.00 MB (55574528)

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Reviewed-on: http://gerrit.cloudera.org:8080/12296
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 7 insertions(+), 2 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 9: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Feb 2019 07:06:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1921/ : 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/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 3
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 30 Jan 2019 01:17:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12296

to look at the new patch set (#3).

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 16 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/3
-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 3
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 1:

(1 comment)

As discussed, we usually use the same CR for subsequent patchsets since the reviewers can easily diff between revisions. Its ok for now though since the patch is small.

http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py@429
PS1, Line 429:     if re.search(" MemoryUsage", profile) is not None:
This is non-deterministic because the query runs so fast that not a single sample is collected. So it is worth running some query which has fragment instances running for > sampling interval (=500ms).

Also since this probably doesn't require a special test, how about moving this assert to after L400? I've checked that the query consistently produces some samples. (lineitem and orders are usually the biggest tables and there is a join + groupby)



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Jan 2019 19:15:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 7:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@13
PS6, Line 13: Add TimeSeriesCounter prefix checks in test_observability tests.
> nit: trailing whitespace
Done


http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@19
PS6, Line 19:         . . .
> I'm not sure whether these ... in the commit message might cause issues for
I used 3 single dots. To be safe, I will add space between them


http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py@404
PS6, Line 404:     assert "  MemoryUsage" not in profile
> I don't think you need a regex here but you could just do "assert pattern i
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 22:30:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12296/4//COMMIT_MSG
Commit Message:

PS4: 
Please explain in the commit message why we make this change (consistency), that only time series counters are affected, and how you tested it. It would also be good to put a small subsection of a profile into the commit message that shows that after this change all counters are consistent.


http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc@777
PS4, Line 777:         stream << prefix << "     - " << v.first << "("
The amount of indent here looks different from the SummaryStatsCounters below, can you please double check that it's correct here? It also seems different from the plain counters in L1155.



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 10:35:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1917/ : 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/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Jan 2019 23:59:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Lars Volker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12296

to look at the new patch set (#5).

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 7 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/5
-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1946/ : 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/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Thu, 31 Jan 2019 22:43:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Lars Volker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12296

to look at the new patch set (#7).

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

The change makes profiles prefix counters consistent.
Only TimeSeriesCounters are affected.

Testing:
Add TimeSeriesCounter prefix checks in test_observability tests.
Manual Tests: Run query and check profile for MemoryUsage and
ThreadUsage. Following section of profile shows that TimeSeriesCounters
are consistent with other counters:

    Fragment F00:
        . . .
        Fragment Instance Lifecycle Event Timeline: 273.841ms
           - Prepare Finished: 1.511ms (1.511ms)
           . . .
         - MemoryUsage(500.000ms): 2.81 MB
         - ThreadUsage(500.000ms): 1
         - AverageThreadTokens: 1.00
         - BloomFilterBytes: 1.00 MB (1048576)
         - ExchangeScanRatio: 0.00
         - PeakMemoryUsage: 54.26 MB (56891933)
         - PeakReservation: 53.00 MB (55574528)

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 7 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/7
-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3703/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 8
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Feb 2019 05:59:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@13
PS6, Line 13: Add TimeSeriesCounter prefix checks in test_observability tests. 
nit: trailing whitespace


http://gerrit.cloudera.org:8080/#/c/12296/6//COMMIT_MSG@19
PS6, Line 19:         …
I'm not sure whether these ... in the commit message might cause issues for some tools since they're not ascii characters. To be on the safe side you could replace them with three single dots.


http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/6/tests/query_test/test_observability.py@404
PS6, Line 404:     assert re.search("  MemoryUsage", profile) is None
I don't think you need a regex here but you could just do "assert pattern in profile" like below



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 19:27:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 6:

(3 comments)

Patch set 6 should address all the issues.

http://gerrit.cloudera.org:8080/#/c/12296/4//COMMIT_MSG
Commit Message:

PS4: 
> Please explain in the commit message why we make this change (consistency),
Done


http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc
File be/src/util/runtime-profile.cc:

http://gerrit.cloudera.org:8080/#/c/12296/4/be/src/util/runtime-profile.cc@777
PS4, Line 777:         stream << prefix << "   - " << v.first << "("
> The amount of indent here looks different from the SummaryStatsCounters bel
I used the same prefix as event timers above. Published new fix to make it the same as SummaryStatsCounter.


http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py@429
PS1, Line 429:   @classmethod
> This is non-deterministic because the query runs so fast that not a single 
For this seq/snam/block format, this query can go into the if statement 100% with my local test. I used the test to setup the format to slow down the join to let the counter appear in the profile. And it is clearer to let more test works for a jira.



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 17:10:34 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1957/ : 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/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 15:29:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Lars Volker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12296

to look at the new patch set (#4).

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 8 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/4
-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 8: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 8
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Feb 2019 06:09:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1960/ : 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/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 16:17:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3711/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Feb 2019 02:59:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12296/5/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/5/tests/query_test/test_observability.py@403
PS5, Line 403: e
flake8: E501 line too long (93 > 90 characters)



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 14:44:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/1/tests/query_test/test_observability.py@424
PS1, Line 424: ,
flake8: E231 missing whitespace after ','



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 29 Jan 2019 21:31:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 9: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Feb 2019 02:59:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 7:

The failure is not related. Builds against other jira patches(1963, 1964) has the same failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Feb 2019 00:23:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Lars Volker, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/12296

to look at the new patch set (#6).

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................

IMPALA-8093: Prefix time series counters with a hyphen

The change makes profiles prefix counters consistent.
Only TimeSeriesCounters are affected.

Testing:
Add TimeSeriesCounter prefix checks in test_observability tests. 
Manual Tests: Run query and check profile for MemoryUsage and
ThreadUsage. Following section of profile shows that TimeSeriesCounters
are consistent with other counters:

    Fragment F00:
        …
        Fragment Instance Lifecycle Event Timeline: 273.841ms
           - Prepare Finished: 1.511ms (1.511ms)
           …
         - MemoryUsage(500.000ms): 2.81 MB
         - ThreadUsage(500.000ms): 1
         - AverageThreadTokens: 1.00 
         - BloomFilterBytes: 1.00 MB (1048576)
         - ExchangeScanRatio: 0.00 
         - PeakMemoryUsage: 54.26 MB (56891933)
         - PeakReservation: 53.00 MB (55574528)

Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
---
M be/src/util/runtime-profile.cc
M tests/query_test/test_observability.py
2 files changed, 7 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/12296/6
-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 8: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3703/


-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 8
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Feb 2019 06:59:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8093: Prefix time series counters with a hyphen

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12296 )

Change subject: IMPALA-8093: Prefix time series counters with a hyphen
......................................................................


Patch Set 4: Code-Review+1

(2 comments)

Lars, can you please do a +2 pass?

http://gerrit.cloudera.org:8080/#/c/12296/4/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/12296/4/tests/query_test/test_observability.py@403
PS4, Line 403: if re.search(" MemoryUsage", profile) is not None:
not needed anymore.


http://gerrit.cloudera.org:8080/#/c/12296/4/tests/query_test/test_observability.py@404
PS4, Line 404: MemoryUsage is a Time series counter, it should start with " - ", not "  "
nit: say something like "Time series counters should be prefixed with a hyphen" ?



-- 
To view, visit http://gerrit.cloudera.org:8080/12296
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e3f08da765b3e6dedead45760729cbc5e8fb6fa
Gerrit-Change-Number: 12296
Gerrit-PatchSet: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Feb 2019 02:43:37 +0000
Gerrit-HasComments: Yes