You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zach Amsden (Code Review)" <ge...@cloudera.org> on 2017/01/19 21:43:20 UTC

[Impala-ASF-CR] Add the query handle to error messages for Invalid Query Handle for beeswax interface.

Zach Amsden has uploaded a new change for review.

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

Change subject: Add the query handle to error messages for Invalid Query Handle for beeswax interface.
......................................................................

Add the query handle to error messages for Invalid Query Handle
for beeswax interface.

Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
---
M be/src/service/impala-beeswax-server.cc
M tests/comparison/leopard/report.py
2 files changed, 3 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>

[Impala-ASF-CR] IMPALA-5748 Record query handle for invalid handle

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-5748 Record query handle for invalid handle
......................................................................


Patch Set 2:

(2 comments)

Do you think the perf difference is worth considering replacing Substitute()? I don't think that Substitute() appears (or should) on perf critical paths, and I gotta say I'm not in love with karma's syntax.

http://gerrit.cloudera.org:8080/#/c/5748/2//COMMIT_MSG
Commit Message:

PS2, Line 7: 5748
I think this is the wrong jira (plus, nitpicking, add a : after the number for consistency).


http://gerrit.cloudera.org:8080/#/c/5748/2/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

Line 38: #include <boost/spirit/include/karma.hpp>
We really try not to expand our boost footprint if at all possible.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has uploaded a new patch set (#4).

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................

IMPALA-3785: Record query handle for invalid handle

Add the query handle to error messages for Invalid Query Handle
for beeswax and HS2 interfaces.

Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/util/debug-util-test.cc
M tests/comparison/leopard/report.py
M tests/hs2/test_hs2.py
5 files changed, 130 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>

[Impala-ASF-CR] Add the query handle to error messages for Invalid Query Handle for beeswax interface.

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Add the query handle to error messages for Invalid Query Handle for beeswax interface.
......................................................................


Patch Set 1:

> I was going to do that but I wasn't sure how to test the HS2 code
 > path.  Also, is looks like there is more conversion to do on the
 > HS2 query handle, looks like it is stored as bytes and the output
 > should most likely be a hex UUID.  We wouldn't want to output the
 > secret part of the handle I am guessing.
 > 
 > Was considering using boost::spirit::karma for that but this
 > doesn't seem to be a common theme in the code yet (it should be -
 > it's rather fast).

The query_id is the non-secret part, which you'll already have in TUniqueId format. See various places in impala-hs2-server.cc  (Also note that we don't actually generate secret -- see ImpalaServer::ExecuteStatement() in impala-hs2-server.cc.)

As far as testing, unfortunately our tests still use beeswax for the most part. But we do have some tests that use hs2 in tests/hs2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 3:

I don't think we should worry about optimizing Substitute().

Zach, how did you test this change?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

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

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


IMPALA-3785: Record query handle for invalid handle

Add the query handle to error messages for Invalid Query Handle
for beeswax and HS2 interfaces.

Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Reviewed-on: http://gerrit.cloudera.org:8080/5748
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/util/debug-util-test.cc
M tests/comparison/leopard/report.py
M tests/hs2/test_hs2.py
5 files changed, 64 insertions(+), 19 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5748/4/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

PS4, Line 193: handle.id
how about PrintId(query_id) for consistency with the other cases? this path isn't perf critical at all.


PS4, Line 253: handle.id
same


http://gerrit.cloudera.org:8080/#/c/5748/4/be/src/util/debug-util-test.cc
File be/src/util/debug-util-test.cc:

Line 81: TEST(DebugUtil, ErrMsgBenchmark) {
Henry mentioned in the previous patchset where benchmarking code usually lives.


http://gerrit.cloudera.org:8080/#/c/5748/4/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

PS4, Line 234: 0123456789abcdef
how about writing these as as hex bytes so that it's obvious that the query id printed below matches exactly? i.e. "\x01\x23\x45 ..."


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add the query handle to error messages for Invalid Query Handle for beeswax interface.

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: Add the query handle to error messages for Invalid Query Handle for beeswax interface.
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5748/1//COMMIT_MSG
Commit Message:

Line 7: Add the query handle to error messages for Invalid Query Handle
prefix with the jira. i.e.:

IMPALA-3785: Add the query handle ...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has uploaded a new patch set (#3).

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................

IMPALA-3785: Record query handle for invalid handle

Add the query handle to error messages for Invalid Query Handle
for beeswax and HS2 interfaces.

Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/util/debug-util-test.cc
M tests/comparison/leopard/report.py
4 files changed, 113 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>

[Impala-ASF-CR] IMPALA-5748 Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.

Change subject: IMPALA-5748 Record query handle for invalid handle
......................................................................


Patch Set 2:

Henry, I don't think it matters at all in this case, but I did want to figure out how to use the unit test and benchmark code.  The boost syntax is pretty ugly.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Add the query handle to error messages for Invalid Query Handle for beeswax interface.

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: Add the query handle to error messages for Invalid Query Handle for beeswax interface.
......................................................................


Patch Set 1:

(1 comment)

Any chance you want to do the same thing for HS2? See impala-hs2-server.cc.

http://gerrit.cloudera.org:8080/#/c/5748/1/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

PS1, Line 193: "Invalid query handle: " + handle.id
try and avoid operator+ for strings - sometimes it doesn't do what you might expect. We prefer 

  Substitute("Invalid query handle: $0", handle.id)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has uploaded a new patch set (#5).

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................

IMPALA-3785: Record query handle for invalid handle

Add the query handle to error messages for Invalid Query Handle
for beeswax and HS2 interfaces.

Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
---
M be/src/service/impala-beeswax-server.cc
M be/src/service/impala-hs2-server.cc
M be/src/util/debug-util-test.cc
M tests/comparison/leopard/report.py
M tests/hs2/test_hs2.py
5 files changed, 64 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 5: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5748/5/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

PS5, Line 234: 76543210
the point of using hex format was to make it trivially easy to see that the returned query id is the same. i guess you'll probably argue that everyone should know digits' ASCII codes.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] Add the query handle to error messages for Invalid Query Handle for beeswax interface.

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.

Change subject: Add the query handle to error messages for Invalid Query Handle for beeswax interface.
......................................................................


Patch Set 1:

I was going to do that but I wasn't sure how to test the HS2 code path.  Also, is looks like there is more conversion to do on the HS2 query handle, looks like it is stored as bytes and the output should most likely be a hex UUID.  We wouldn't want to output the secret part of the handle I am guessing.

Was considering using boost::spirit::karma for that but this doesn't seem to be a common theme in the code yet (it should be - it's rather fast).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5748 Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.

Change subject: IMPALA-5748 Record query handle for invalid handle
......................................................................


Patch Set 1:

So I wrote a little benchmark.  As expected, karma blew away all the other forms of string concatenation.  Will come up with a nicer syntax for this but likely it will need to be a macro.

impala@impala-dev:~/Impala$ ./be/build/debug/util/debug-util-test
[==========] Running 5 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 5 tests from DebugUtil
[ RUN      ] DebugUtil.UniqueID
[       OK ] DebugUtil.UniqueID (0 ms)
[ RUN      ] DebugUtil.ErrMsgBenchmark
Rate Append: 1750.86
Rate Subst: 934.806
Rate Gen: 379.784
[       OK ] DebugUtil.ErrMsgBenchmark (165 ms)
[ RUN      ] DebugUtil.StackDump

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5748 Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has uploaded a new patch set (#2).

Change subject: IMPALA-5748 Record query handle for invalid handle
......................................................................

IMPALA-5748 Record query handle for invalid handle

Add the query handle to error messages for Invalid Query Handle
for beeswax and HS2 interfaces.

Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
---
M be/src/service/impala-beeswax-server.cc
M be/src/util/debug-util-test.cc
M tests/comparison/leopard/report.py
3 files changed, 80 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/48/5748/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5748
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 5:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/236/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5748/3/be/src/util/debug-util-test.cc
File be/src/util/debug-util-test.cc:

Line 81: TEST(DebugUtil, ErrMsgBenchmark) {
If you want to keep this around, look in be/src/benchmarks - that's where we keep benchmark code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3785: Record query handle for invalid handle

Posted by "Zach Amsden (Code Review)" <ge...@cloudera.org>.
Zach Amsden has posted comments on this change.

Change subject: IMPALA-3785: Record query handle for invalid handle
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5748/4/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

PS4, Line 193: PrintId(q
> how about PrintId(query_id) for consistency with the other cases? this path
Done


PS4, Line 253: PrintId(q
> same
Done


http://gerrit.cloudera.org:8080/#/c/5748/5/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

PS5, Line 234: 76543210
> the point of using hex format was to make it trivially easy to see that the
It's actually non-trivial anyway because of the way the endian-ness comes out when converting from a "string" to a hex printed handle.  This was the best way I could find to try to make that clear.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc113b3673e1b90f81e80e841740b8006bfd31ba
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <za...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-HasComments: Yes