Test App: Add Java code for transaction retries #987

rjnn · 2017-01-11T22:10:18Z

Add example code for transaction retry logic. Compilation and
execution instructions are in a comment in the main code file.

This change is

cockroach-teamcity · 2017-01-11T22:28:16Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/3e91e90aa95f1b86d83cd446073764e22e8b8ba9/

BramGruneir · 2017-01-12T16:42:24Z

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

_includes/app/TxnSample.java, line 19 at r1 (raw file):

    public static RetryableTransaction transferFunds(int from, int to, int amount) {
        return new RetryableTransaction() {
            public void run(Connection conn) throws Exception {

Won't this only throw SQLException and InsufficientBalanceException? Why not just list only those two?

_includes/app/TxnSample.java, line 21 at r1 (raw file):

            public void run(Connection conn) throws Exception {
                ResultSet res = conn.createStatement().executeQuery("SELECT balance FROM accounts WHERE id = " + from);
                res.next();

Why do you need the next here? Shouldn't you already be at the first result?
What does the resultSet look like? Should you check to make sure it returns true?

_includes/app/TxnSample.java, line 32 at r1 (raw file):

    }

    public static void retryTransaction(Connection conn, RetryableTransaction tx) throws Exception {

Again here, list only the exceptions that you're going to throw.

_includes/app/TxnSample.java, line 66 at r1 (raw file):

                db.setAutoCommit(false);

                RetryableTransaction transfer = transferFunds(1, 2, 100);

So this assumes there's already a db with a table, with two accounts setup. Perhaps add a comment about the initial setup above.

_includes/app/TxnSample.java, line 72 at r1 (raw file):

                    System.out.printf("\taccount %s: %s\n", res.getInt("id"), res.getInt("balance"));
                }
            } catch(InsufficientBalanceException ibe) {

From what I've seen, these are all usually named just e. Unless you're got nested try catches. And above, you just use e.

Comments from Reviewable

rjnn · 2017-01-12T16:50:06Z

Thank you for the review!

Review status: 2 of 3 files reviewed at latest revision, 5 unresolved discussions.

_includes/app/TxnSample.java, line 19 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Won't this only throw SQLException and InsufficientBalanceException? Why not just list only those two?

Done.

_includes/app/TxnSample.java, line 21 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Why do you need the next here? Shouldn't you already be at the first result?
What does the resultSet look like? Should you check to make sure it returns true?

The Java ResultSet spec says A ResultSet cursor is initially positioned before the first row; the first call to the method next makes the first row the current row.
Added a check that it returns true.

_includes/app/TxnSample.java, line 32 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

Again here, list only the exceptions that you're going to throw.

Done.

_includes/app/TxnSample.java, line 66 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

So this assumes there's already a db with a table, with two accounts setup. Perhaps add a comment about the initial setup above.

Added.

_includes/app/TxnSample.java, line 72 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

From what I've seen, these are all usually named just e. Unless you're got nested try catches. And above, you just use e.

Done.

Comments from Reviewable

cockroach-teamcity · 2017-01-12T16:51:18Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/206623d2b3fb5f1d4302f4e909a6d83aa2d34911/

BramGruneir · 2017-01-12T16:55:59Z

A few final nits, but looks good.

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

_includes/app/TxnSample.java, line 21 at r1 (raw file):

Previously, arjunravinarayan (Arjun Narayan) wrote…

The Java ResultSet spec says A ResultSet cursor is initially positioned before the first row; the first call to the method next makes the first row the current row.
Added a check that it returns true.

If you want the first element, why not use first? And on failure, you probably want another exception, maybe a accountNotFoundException (that takes an account number) or something like that to match the insufficientBalanceException.

Comments from Reviewable

rjnn · 2017-01-12T17:03:45Z

Thank you!

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion.

_includes/app/TxnSample.java, line 21 at r1 (raw file):

Previously, BramGruneir (Bram Gruneir) wrote…

If you want the first element, why not use first? And on failure, you probably want another exception, maybe a accountNotFoundException (that takes an account number) or something like that to match the insufficientBalanceException.

The previous example program in the docs uses next() for this so consistency. I'm ambivalent either way.

Added the other exception. Even added a constructor for it and everything, just for MAXIMUM JAVANESS. Let me know if you would like an ExceptionFactory too :)

Comments from Reviewable

rjnn · 2017-01-12T17:05:31Z

@jseldess happy to make stylistic changes as you would like, or add/improve the comments. I'll wait for your approval before merging.

cockroach-teamcity · 2017-01-12T17:05:57Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/640411aba36fe3fb4d150f68b6b3998e67fa6f7a/

jseldess

LGTM. Just some suggestions to add comments similar to our other samples. Our commenting eventually needs to be made more consistent across samples, but this should be good for now.

jseldess · 2017-01-12T16:56:46Z

build-a-test-app.md

-</div>
+~~~ java
+{% include app/TxnSample.java %}
+~~~



We need to put the closing </div> back in. Otherwise, the other transaction samples don't show up.

jseldess · 2017-01-12T17:06:56Z

_includes/app/TxnSample.java

+                if(balance < from) {
+                    throw new InsufficientBalanceException();
+                }
+                conn.createStatement().executeUpdate("UPDATE accounts SET balance = balance - " + amount + " where id = " + from);


Maybe add comment before line 28: # Perform the transfer.

jseldess · 2017-01-12T17:07:23Z

_includes/app/TxnSample.java

+    public static RetryableTransaction transferFunds(int from, int to, int amount) {
+        return new RetryableTransaction() {
+            public void run(Connection conn) throws SQLException, InsufficientBalanceException {
+                ResultSet res = conn.createStatement().executeQuery("SELECT balance FROM accounts WHERE id = " + from);


Maybe add comment before line 20: # Check the current balance.

jseldess · 2017-01-12T17:07:39Z

_includes/app/TxnSample.java

+                // This assumes the table has already been set up as in the "Build a Test App" tutorial.
+                RetryableTransaction transfer = transferFunds(1, 2, 100);
+                retryTransaction(db, transfer);
+                ResultSet res = db.createStatement().executeQuery("SELECT id, balance FROM accounts");


Maybe add comment before line 70: # Check balances after transfer.

BramGruneir · 2017-01-12T17:15:25Z

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

_includes/app/TxnSample.java, line 21 at r1 (raw file):

Previously, arjunravinarayan (Arjun Narayan) wrote…

The previous example program in the docs uses next() for this so consistency. I'm ambivalent either way.

Added the other exception. Even added a constructor for it and everything, just for MAXIMUM JAVANESS. Let me know if you would like an ExceptionFactory too :)

For MAXIMUM JAVANESS, you need to add an abstract factory for that factory and use dependency injection to inform it at runtime as to what type of factory to make.

Comments from Reviewable

Add example code for transaction retry logic. Compilation and execution instructions are in a comment in the main code file. Fixes cockroachdb#757.

rjnn · 2017-01-12T18:13:16Z

Review status: 1 of 3 files reviewed at latest revision, 4 unresolved discussions.

build-a-test-app.md, line 219 at r2 (raw file):

Previously, jseldess wrote…

We need to put the closing </div> back in. Otherwise, the other transaction samples don't show up.

Done.

_includes/app/TxnSample.java, line 20 at r2 (raw file):

Previously, jseldess wrote…

Maybe add comment before line 20: # Check the current balance.

Done.

_includes/app/TxnSample.java, line 28 at r2 (raw file):

Previously, jseldess wrote…

Maybe add comment before line 28: # Perform the transfer.

Done.

_includes/app/TxnSample.java, line 70 at r2 (raw file):

Previously, jseldess wrote…

Maybe add comment before line 70: # Check balances after transfer.

Done.

Comments from Reviewable

cockroach-teamcity · 2017-01-12T18:14:22Z

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/d0573ef067aafb0c62b5a0829efc03d86fb14f34/

rjnn assigned BramGruneir and jseldess Jan 11, 2017

rjnn requested a review from BramGruneir January 11, 2017 22:10

rjnn force-pushed the add_java_txn_example_code branch 2 times, most recently from fa6cc52 to 3e91e90 Compare January 11, 2017 22:27

rjnn force-pushed the add_java_txn_example_code branch from 3e91e90 to 206623d Compare January 12, 2017 16:49

rjnn force-pushed the add_java_txn_example_code branch from 206623d to 640411a Compare January 12, 2017 17:03

rjnn unassigned BramGruneir Jan 12, 2017

jseldess approved these changes Jan 12, 2017

View reviewed changes

Test App: Add Java code for transaction retries

d0573ef

Add example code for transaction retry logic. Compilation and execution instructions are in a comment in the main code file. Fixes cockroachdb#757.

rjnn force-pushed the add_java_txn_example_code branch from 640411a to d0573ef Compare January 12, 2017 18:12

rjnn merged commit b012250 into cockroachdb:gh-pages Jan 12, 2017

rjnn deleted the add_java_txn_example_code branch January 12, 2017 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test App: Add Java code for transaction retries #987

Test App: Add Java code for transaction retries #987

rjnn commented Jan 11, 2017 •

edited by andreimatei

Loading

cockroach-teamcity commented Jan 11, 2017

BramGruneir commented Jan 12, 2017

rjnn commented Jan 12, 2017

cockroach-teamcity commented Jan 12, 2017

BramGruneir commented Jan 12, 2017

rjnn commented Jan 12, 2017

rjnn commented Jan 12, 2017

cockroach-teamcity commented Jan 12, 2017

jseldess left a comment

jseldess Jan 12, 2017

jseldess Jan 12, 2017

jseldess Jan 12, 2017

jseldess Jan 12, 2017

BramGruneir commented Jan 12, 2017

rjnn commented Jan 12, 2017

cockroach-teamcity commented Jan 12, 2017

Test App: Add Java code for transaction retries #987

Test App: Add Java code for transaction retries #987

Conversation

rjnn commented Jan 11, 2017 • edited by andreimatei Loading

cockroach-teamcity commented Jan 11, 2017

BramGruneir commented Jan 12, 2017

rjnn commented Jan 12, 2017

cockroach-teamcity commented Jan 12, 2017

BramGruneir commented Jan 12, 2017

rjnn commented Jan 12, 2017

rjnn commented Jan 12, 2017

cockroach-teamcity commented Jan 12, 2017

jseldess left a comment

Choose a reason for hiding this comment

jseldess Jan 12, 2017

Choose a reason for hiding this comment

jseldess Jan 12, 2017

Choose a reason for hiding this comment

jseldess Jan 12, 2017

Choose a reason for hiding this comment

jseldess Jan 12, 2017

Choose a reason for hiding this comment

BramGruneir commented Jan 12, 2017

rjnn commented Jan 12, 2017

cockroach-teamcity commented Jan 12, 2017

rjnn commented Jan 11, 2017 •

edited by andreimatei

Loading