[kudu-CR](branch-1.4.x) KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

2017-10-06 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8224 )

Change subject: KUDU-2167: fix C++ client crash due to bad assumption regarding 
scan data
..


Patch Set 1: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: comment
Gerrit-Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Gerrit-Change-Number: 8224
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Comment-Date: Fri, 06 Oct 2017 16:43:54 +
Gerrit-HasComments: No


[kudu-CR](branch-1.4.x) KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

2017-10-06 Thread Jean-Daniel Cryans (Code Review)
Jean-Daniel Cryans has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8224 )

Change subject: KUDU-2167: fix C++ client crash due to bad assumption regarding 
scan data
..

KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

The new unit test triggered the crash reliably, though it's probably not the
only way (or perhaps not even the best way) to trigger it.

I also modified the tserver to always populate the scan data fields so that
older clients are also protected. This necessitated an additional change to
how a scanner's "data in open" was computed: the existence of the 'data'
field in the scan response isn't enough. The Java client was already doing
this, so it seems like a reasonable change to make on the C++ side.

Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Reviewed-on: http://gerrit.cloudera.org:8080/8204
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
(cherry picked from commit 858bf73b380d10f16e7aca04a780a0adae705e9a)
Reviewed-on: http://gerrit.cloudera.org:8080/8224
Tested-by: Adar Dembo 
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/client/client-test.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/tserver/tablet_service.cc
3 files changed, 69 insertions(+), 45 deletions(-)

Approvals:
  Adar Dembo: Verified
  Jean-Daniel Cryans: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: merged
Gerrit-Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Gerrit-Change-Number: 8224
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 


[kudu-CR](branch-1.4.x) KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

2017-10-05 Thread Adar Dembo (Code Review)
Adar Dembo has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/8224 )

Change subject: KUDU-2167: fix C++ client crash due to bad assumption regarding 
scan data
..


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Gerrit-Change-Number: 8224
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 


[kudu-CR](branch-1.4.x) KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

2017-10-05 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8224 )

Change subject: KUDU-2167: fix C++ client crash due to bad assumption regarding 
scan data
..


Patch Set 1: Verified+1

Overriding Jenkins, known flaky test.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: comment
Gerrit-Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Gerrit-Change-Number: 8224
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Fri, 06 Oct 2017 06:23:28 +
Gerrit-HasComments: No


[kudu-CR](branch-1.4.x) KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

2017-10-05 Thread Adar Dembo (Code Review)
Adar Dembo has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8224


Change subject: KUDU-2167: fix C++ client crash due to bad assumption regarding 
scan data
..

KUDU-2167: fix C++ client crash due to bad assumption regarding scan data

The new unit test triggered the crash reliably, though it's probably not the
only way (or perhaps not even the best way) to trigger it.

I also modified the tserver to always populate the scan data fields so that
older clients are also protected. This necessitated an additional change to
how a scanner's "data in open" was computed: the existence of the 'data'
field in the scan response isn't enough. The Java client was already doing
this, so it seems like a reasonable change to make on the C++ side.

Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Reviewed-on: http://gerrit.cloudera.org:8080/8204
Reviewed-by: Todd Lipcon 
Tested-by: Kudu Jenkins
(cherry picked from commit 858bf73b380d10f16e7aca04a780a0adae705e9a)
---
M src/kudu/client/client-test.cc
M src/kudu/client/scanner-internal.cc
M src/kudu/tserver/tablet_service.cc
3 files changed, 69 insertions(+), 45 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/8224/1
--
To view, visit http://gerrit.cloudera.org:8080/8224
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.4.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: If1a8a4e22082cf39710b9f00894f644a0b34234e
Gerrit-Change-Number: 8224
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo