Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1767. Add java test for client operation interleaving ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/5465/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSessionInterleaving.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSessionInterleaving.java: Line 19: import java.util.ArrayList; Your imports are in the wrong order, use the kudu_style.xml file to see what's wrong. Line 64: final String TABLE_NAME = "test-table"; Usually we try to use the test's name for the table name. Also, can you extract all those final fields? Line 65: final long TIMEOUT_MILLIS = 30000; We have a DEFAULT_SLEEP that comes from BaseKuduTest that we use, but based on my syncClient comment below you shouldn't even need to sleep. Line 74: AsyncKuduSession session = client.newSession(); I'd encourage you to use the syncClient, since you're not making use of any specific async feature in this test. It will also simplify your test. Line 102: Thread.sleep(THROTTLE_SLEEP_MS); FYI the way to handle this is to join on the Deferred provided with the exception, this way you just wait what you need to wait. Line 115: for (Deferred<OperationResponse> d : applyDeferreds) { Doing this pretty much voids your warranty in the async client with AUTO_FLUSH_SYNC, the client doesn't guarantee correct ordering between calls if you're not waiting for each operation to finish before sending the next one. For correct batching you'd want MANUAL_FLUSH or AUTO_FLUSH_BACKGROUND. You might be hitting this issue when your test fails, instead of the server-side issue you're trying to trigger. Using the sync client avoids this problem. Line 135: To be clean, with AUTO_FLUSH_BACKGROUND you should also check the error collector. -- To view, visit http://gerrit.cloudera.org:8080/5465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd30aeed34548b01ba4ad9b08cfd5fae22f967e9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-HasComments: Yes
