You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/04/24 23:30:20 UTC

[kudu-CR] java: prohibit use of a KuduTable from an unassociated KuduClient

Hello Alexey Serbin, David Ribeiro Alves, Jean-Daniel Cryans, Kudu Jenkins, Anonymous Coward #314, 

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

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

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

Change subject: java: prohibit use of a KuduTable from an unassociated KuduClient
......................................................................

java: prohibit use of a KuduTable from an unassociated KuduClient

This fixes a request-tracking issue with the following code
anti-pattern in which a KuduTable associated with one client is used to
create operations applied to another client's session:

  KuduClient client1 = KuduClientBuilder....newClient();
  KuduTable t = client1.openTable(...);
  KuduClient client2 = KuduClientBuilder....newClient();
  KuduSession s = client2.newSession();
  s.apply(t.newUpdate());

This would cause sequence numbers to be generated out of the session's
client's RequestTracker, but then marked complete in the operation's
client's RequestTracker. This could cause any number of issues:

- a cache "hit" on the server side might cause an operation to get an
  unrelated operation's response
- a cache "miss" on the server side might result in an operation
  incorrectly being marked as already-responded and garbage collected,
  causing it to fail.

This patch adds a Preconditions check for this issue and fixes some tests
where it was triggered.

Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/RequestTracker.java
M java/kudu-client/src/test/java/org/apache/kudu/client/ITClientStress.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestRequestTracker.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestScannerMultiTablet.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
9 files changed, 103 insertions(+), 57 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/7362/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4a977f36b9c7ba3758322a2216ce90208b5d014
Gerrit-Change-Number: 7362
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward #314
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>