You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2017/06/23 20:53:46 UTC

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0 

Joe McDonnell has uploaded a new change for review.

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................

IMPALA-4687: Get Impala working against HBase 2.0

This changes Impala code to tolerate the API differences
between HBase 1.0 and HBase 2.0. It also drops
compatibility code for older HBase versions.

Specific changes:
1. Tolerate return value of Scan for Scan.setCaching()
and Scan.setCacheBlocks(). This has no impact on our code.
2. HBase 2.0 eliminates the ScannerTimeoutException. The
case that previously generated the exception will now
recreate the scanner, so it is not necessary for our code
to recreate the scanner. Short-circuit
HandleResultScannerTimeout on HBase 2.0.
3. HBase 2.0 eliminates the Put.add(), which has been
replaced with Put.addColumn(). This API exists in
HBase 1.0, so it is safe to switch this completely.

This was tested by verifying that an HBase 2.0 cluster
starts up.

Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
---
M be/src/exec/hbase-table-scanner.cc
M be/src/exec/hbase-table-scanner.h
M be/src/exec/hbase-table-writer.cc
M be/src/exec/hbase-table-writer.h
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
6 files changed, 82 insertions(+), 64 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 1:

(13 comments)

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

PS1, Line 25: This was tested by verifying that an HBase 2.0 cluster
            : starts up.
> I think we should run hbase stress test eventually to test the new ScannerT
I think we can run stress tests with existing code. Since HBase stopped throwing this exception with HBASE-16266, I think this is already available in versions of HBase 1.2, which we already support.


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS1, Line 238: 
             : 
> May be retain this part.
Done


PS1, Line 197: setStartRow
> You mentioned on the jira that these are deprecated too (setStartRow/setSto
It turns out that setStartRow/setStopRow are newly deprecated. The new methods (with*Row) do not exist in HBase 1.2, so we can't replace it without having conditional code to detect the different APIs. I don't think it is worth switching yet. I will file a separate JIRA.


PS1, Line 403: jthrowable exc = env->ExceptionOccurred();
             :   if (exc == NULL) return Status::OK();
> Unrelated to your change, but why are we calling this here? Shouldn't GetJn
We need the actual jthrowable so that we can check whether it is an instance of ScannerTimeoutException. GetJniExceptionMsg only returns the message.


Line 407:   // necessary). For HBase 2.0, ScannerTimeoutException does not exist, so simple
> typo: simply
Done


PS1, Line 407: simple
> simply?
Done


Line 411:   if (scanner_timeout_ex_cl_ == nullptr) return status;
> Is there new exception that signals we need to retry? Is the retry not nece
From what I understand, HBase does a heartbeat now, and where it previously would throw this exception, it now resets the scanner and retries. HBASE-16266 is when this changed.


Line 512:       // Newer versions of HBase re-create the ResultScanner without throwing this
> Is "re-create" the right term? I'm not sure how that would work. Do you mea
Reworked this comment.


Line 514:       // code will simply return the error.
> what is "the error"?
Fixed this language.


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.h
File be/src/exec/hbase-table-scanner.h:

Line 61: /// 1. Scan.setCaching and Scan.setCacheBlocks tolerate the older void return value
> use () for function/methods, e.g. Scan.setCaching()
Done


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-writer.h
File be/src/exec/hbase-table-writer.h:

PS1, Line 108: add
> addColumn
Done


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:

Line 61:   env->GetMethodID(class_ref, method_str, method_signature);
> Reading the documentation for "GetMethodID"
Good point. I haven't looked at the return value, but it does throw NoSuchMethodException. Maybe it also returns null. I think either would work, but in either case, we would need to clear the exception.


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/util/jni-util.h
File be/src/util/jni-util.h:

PS1, Line 171: a method
> ..non static method..
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 1:

(8 comments)

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

PS1, Line 25: This was tested by verifying that an HBase 2.0 cluster
            : starts up.
I think we should run hbase stress test eventually to test the new ScannerTimeOutException code paths as it is only retried during load.


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS1, Line 238: 
             : 
May be retain this part.


PS1, Line 197: setStartRow
You mentioned on the jira that these are deprecated too (setStartRow/setStopRow), may be switch to with*Row() methods now that we are here?


PS1, Line 403: jthrowable exc = env->ExceptionOccurred();
             :   if (exc == NULL) return Status::OK();
Unrelated to your change, but why are we calling this here? Shouldn't GetJniExceptionMsg() do the same?


PS1, Line 407: simple
simply?


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-writer.h
File be/src/exec/hbase-table-writer.h:

PS1, Line 108: add
addColumn


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/util/jni-util.cc
File be/src/util/jni-util.cc:

Line 61:   env->GetMethodID(class_ref, method_str, method_signature);
Reading the documentation for "GetMethodID"

"Returns a method ID, or NULL if the specified method cannot be found."

Can't we just do a NULL check on GetMethodID() call?


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/util/jni-util.h
File be/src/util/jni-util.h:

PS1, Line 171: a method
..non static method..


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


IMPALA-4687: Get Impala working against HBase 2.0

This changes Impala code to tolerate the API differences
between HBase 1.0 and HBase 2.0. It also drops
compatibility code for older HBase versions.

Specific changes:
1. Tolerate return value of Scan for Scan.setCaching()
and Scan.setCacheBlocks(). This has no impact on our code.
2. HBase 2.0 eliminates the ScannerTimeoutException. The
case that previously generated the exception will now
recreate the scanner, so it is not necessary for our code
to recreate the scanner. Short-circuit
HandleResultScannerTimeout on HBase 2.0.
3. HBase 2.0 eliminates the Put.add(), which has been
replaced with Put.addColumn(). This API exists in
HBase 1.0, so it is safe to switch this completely.

This was tested by verifying that an HBase 2.0 cluster
starts up.

Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Reviewed-on: http://gerrit.cloudera.org:8080/7277
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/exec/hbase-table-scanner.cc
M be/src/exec/hbase-table-scanner.h
M be/src/exec/hbase-table-writer.cc
M be/src/exec/hbase-table-writer.h
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
6 files changed, 86 insertions(+), 65 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

Line 407:   // necessary). For HBase 2.0, ScannerTimeoutException does not exist, so simple
typo: simply


Line 411:   if (scanner_timeout_ex_cl_ == nullptr) return status;
Is there new exception that signals we need to retry? Is the retry not necessary any more?


Line 512:       // Newer versions of HBase re-create the ResultScanner without throwing this
Is "re-create" the right term? I'm not sure how that would work. Do you mean the ResultScanner internally resets its state to automatically retry?


Line 514:       // code will simply return the error.
what is "the error"?


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.h
File be/src/exec/hbase-table-scanner.h:

Line 61: /// 1. Scan.setCaching and Scan.setCacheBlocks tolerate the older void return value
use () for function/methods, e.g. Scan.setCaching()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 4:

> Please rebase and the I can submit for gvo.

This is rebased all the way.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................

IMPALA-4687: Get Impala working against HBase 2.0

This changes Impala code to tolerate the API differences
between HBase 1.0 and HBase 2.0. It also drops
compatibility code for older HBase versions.

Specific changes:
1. Tolerate return value of Scan for Scan.setCaching()
and Scan.setCacheBlocks(). This has no impact on our code.
2. HBase 2.0 eliminates the ScannerTimeoutException. The
case that previously generated the exception will now
recreate the scanner, so it is not necessary for our code
to recreate the scanner. Short-circuit
HandleResultScannerTimeout on HBase 2.0.
3. HBase 2.0 eliminates the Put.add(), which has been
replaced with Put.addColumn(). This API exists in
HBase 1.0, so it is safe to switch this completely.

This was tested by verifying that an HBase 2.0 cluster
starts up.

Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
---
M be/src/exec/hbase-table-scanner.cc
M be/src/exec/hbase-table-scanner.h
M be/src/exec/hbase-table-writer.cc
M be/src/exec/hbase-table-writer.h
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
6 files changed, 89 insertions(+), 65 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada,

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

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

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................

IMPALA-4687: Get Impala working against HBase 2.0

This changes Impala code to tolerate the API differences
between HBase 1.0 and HBase 2.0. It also drops
compatibility code for older HBase versions.

Specific changes:
1. Tolerate return value of Scan for Scan.setCaching()
and Scan.setCacheBlocks(). This has no impact on our code.
2. HBase 2.0 eliminates the ScannerTimeoutException. The
case that previously generated the exception will now
recreate the scanner, so it is not necessary for our code
to recreate the scanner. Short-circuit
HandleResultScannerTimeout on HBase 2.0.
3. HBase 2.0 eliminates the Put.add(), which has been
replaced with Put.addColumn(). This API exists in
HBase 1.0, so it is safe to switch this completely.

This was tested by verifying that an HBase 2.0 cluster
starts up.

Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
---
M be/src/exec/hbase-table-scanner.cc
M be/src/exec/hbase-table-scanner.h
M be/src/exec/hbase-table-writer.cc
M be/src/exec/hbase-table-writer.h
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
6 files changed, 90 insertions(+), 65 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 4:

Please rebase and the I can submit for gvo.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Alex Behm, Dan Hecht,

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

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

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................

IMPALA-4687: Get Impala working against HBase 2.0

This changes Impala code to tolerate the API differences
between HBase 1.0 and HBase 2.0. It also drops
compatibility code for older HBase versions.

Specific changes:
1. Tolerate return value of Scan for Scan.setCaching()
and Scan.setCacheBlocks(). This has no impact on our code.
2. HBase 2.0 eliminates the ScannerTimeoutException. The
case that previously generated the exception will now
recreate the scanner, so it is not necessary for our code
to recreate the scanner. Short-circuit
HandleResultScannerTimeout on HBase 2.0.
3. HBase 2.0 eliminates the Put.add(), which has been
replaced with Put.addColumn(). This API exists in
HBase 1.0, so it is safe to switch this completely.

This was tested by verifying that an HBase 2.0 cluster
starts up.

Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
---
M be/src/exec/hbase-table-scanner.cc
M be/src/exec/hbase-table-scanner.h
M be/src/exec/hbase-table-writer.cc
M be/src/exec/hbase-table-writer.h
M be/src/util/jni-util.cc
M be/src/util/jni-util.h
6 files changed, 86 insertions(+), 65 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7277/3/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS3, Line 516:       // Newer versions of HBase use a heartbeat, so they do not timeout.
             :       // Rather than throwing the ScannerTimeoutException, HBase will reset
             :       // the scanner and retry. For HBase 2.0, the ScannerTimeoutException is
             :       // removed. In these cases, HandleResultScannerTimeout will return the exception
             :       // error message in the status.
> this comment seems gratuitous to me (already explained in better locations 
Good point. Removed.


http://gerrit.cloudera.org:8080/#/c/7277/3/be/src/exec/hbase-table-scanner.h
File be/src/exec/hbase-table-scanner.h:

PS3, Line 59: supports HBase < 0.95.2
> I guess the implication is that 0.95.2 uses the same API as 1.0?
Modified the comment to note the reason. This is because we get results via Cell class rather than KeyValue class.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS1, Line 197: setStartRow
> Ok, thanks. Mind adding a TODO with jira reference?
Done


http://gerrit.cloudera.org:8080/#/c/7277/2/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS2, Line 411: error
> nit: Don't think error is the right term here. GetJniExceptionMsg() can jus
In this particular case, we already checked that there is an exception, so it won't return Status::OK().


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.h
File be/src/exec/hbase-table-scanner.h:

PS1, Line 65: HBASE-17809.
            : ///    If ScannerTimeout
> nit: Same comment as in the .cc file. We don't always throw an error, we ca
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 3: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 3: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7277/3/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS3, Line 516:       // Newer versions of HBase use a heartbeat, so they do not timeout.
             :       // Rather than throwing the ScannerTimeoutException, HBase will reset
             :       // the scanner and retry. For HBase 2.0, the ScannerTimeoutException is
             :       // removed. In these cases, HandleResultScannerTimeout will return the exception
             :       // error message in the status.
this comment seems gratuitous to me (already explained in better locations -- here, the caller doesn't really care about this detail and whether timeout can be true or not for the given implementation). i.e. you've already hidden the difference inside HandleResultScannerTimeout(), which has common semantics for 1.0 and 2.0.


http://gerrit.cloudera.org:8080/#/c/7277/3/be/src/exec/hbase-table-scanner.h
File be/src/exec/hbase-table-scanner.h:

PS3, Line 59: supports HBase < 0.95.2
I guess the implication is that 0.95.2 uses the same API as 1.0?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 2: Code-Review+1

(4 comments)

http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS1, Line 197: setStartRow
> It turns out that setStartRow/setStopRow are newly deprecated. The new meth
Ok, thanks. Mind adding a TODO with jira reference?


PS1, Line 403: 
             : Status HBaseTableScanner::HandleResultS
> We need the actual jthrowable so that we can check whether it is an instanc
Yea, I missed that part, thanks


http://gerrit.cloudera.org:8080/#/c/7277/2/be/src/exec/hbase-table-scanner.cc
File be/src/exec/hbase-table-scanner.cc:

PS2, Line 411: error
nit: Don't think error is the right term here. GetJniExceptionMsg() can just return Status:OK() as well.


http://gerrit.cloudera.org:8080/#/c/7277/1/be/src/exec/hbase-table-scanner.h
File be/src/exec/hbase-table-scanner.h:

PS1, Line 65: HBASE-17809.
            : ///    If ScannerTimeout
nit: Same comment as in the .cc file. We don't always throw an error, we can just pass if there is no other exception. May be good to clear it up


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4687: Get Impala working against HBase 2.0

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

Change subject: IMPALA-4687: Get Impala working against HBase 2.0
......................................................................


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I87610e25c01b3547ec332c6975b61284b6837d27
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: No