[kudu] 02/02: [java] log warning if applying operation on a closed session

[ad...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 9443a69b0d81c65a174874661e90a13e5792d55b
Author: abc549825 <ab...@163.com>
AuthorDate: Wed Jan 9 19:23:37 2019 +0800

    [java] log warning if applying operation on a closed session
    
    Closed sessions won't flush on another close() call, so it's somewhat
    dubious to apply() an operation to a closed session. This change adds a
    warning if apply() is used with a closed session.
    
    Originally this patch enforced the new behavior via precondition, but that's
    a breaking change for clients that relied on the old broken behavior.
    
    Change-Id: I0fe221fedbb91959985f5ee374f1b691be2426a9
    Reviewed-on: http://gerrit.cloudera.org:8080/12237
    Tested-by: Kudu Jenkins
    Reviewed-by: Grant Henke <gr...@apache.org>
---
 .../org/apache/kudu/client/AsyncKuduSession.java   |  5 ++++
 .../org/apache/kudu/client/TestKuduClient.java     | 27 ++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
index 94ca290..7d649bd 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
@@ -532,6 +532,11 @@ public class AsyncKuduSession implements SessionConfiguration {
     Preconditions.checkArgument(operation.getTable().getAsyncClient() == client,
         "Applied operations must be created from a KuduTable instance opened " +
         "from the same client that opened this KuduSession");
+    if (closed) {
+      // Ideally this would be a precondition, but that may break existing
+      // clients who have grown to rely on this unsafe behavior.
+      LOG.warn("Applying an operation in a closed session; this is unsafe");
+    }
 
     // Freeze the row so that the client can not concurrently modify it while it is in flight.
     operation.getRow().freeze();
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
index 97df3e0..4584890 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
@@ -1071,7 +1071,6 @@ public class TestKuduClient {
               session.apply(insert);
             }
             session.flush();
-            session.close();
 
             // Perform a bunch of READ_YOUR_WRITES scans to all the replicas
             // that count the rows. And verify that the count of the rows
@@ -1114,7 +1113,7 @@ public class TestKuduClient {
       future.get();
     }
   }
-  
+
   private void runTestCallDuringLeaderElection(String clientMethodName) throws Exception {
     // This bit of reflection helps us avoid duplicating test code.
     Method methodToInvoke = KuduClient.class.getMethod(clientMethodName);
@@ -1138,6 +1137,7 @@ public class TestKuduClient {
                     .build();
     try {
       methodToInvoke.invoke(cl);
+      fail();
     } catch (InvocationTargetException ex) {
       assertTrue(ex.getTargetException() instanceof KuduException);
       KuduException realEx = (KuduException)ex.getTargetException();
@@ -1154,6 +1154,7 @@ public class TestKuduClient {
   public void testGetHiveMetastoreConfigDuringLeaderElection() throws Exception {
     runTestCallDuringLeaderElection("getHiveMetastoreConfig");
   }
+
   /**
    * Test assignment of a location to the client.
    */
@@ -1173,4 +1174,26 @@ public class TestKuduClient {
     client.listTabletServers();
     assertEquals("/L0", client.getLocationString());
   }
+
+  @Test(timeout = 100000)
+  public void testSessionOnceClosed() throws Exception {
+    client.createTable(TABLE_NAME, basicSchema, getBasicCreateTableOptions());
+    KuduTable table = client.openTable(TABLE_NAME);
+    KuduSession session = client.newSession();
+
+    session.setFlushMode(SessionConfiguration.FlushMode.MANUAL_FLUSH);
+    Insert insert = createBasicSchemaInsert(table, 0);
+    session.apply(insert);
+    session.close();
+    assertTrue(session.isClosed());
+
+    insert = createBasicSchemaInsert(table, 1);
+    CapturingLogAppender cla = new CapturingLogAppender();
+    try (Closeable c = cla.attach()) {
+      session.apply(insert);
+    }
+    String loggedText = cla.getAppendedText();
+    assertTrue("Missing warning:\n" + loggedText,
+               loggedText.contains("this is unsafe"));
+  }
 }