Update Hibernate example with app-level retry loop #5760

rmloveland · 2019-10-31T20:18:38Z

cockroach-teamcity · 2019-10-31T20:18:44Z

This change is

cockroach-teamcity · 2019-10-31T20:21:12Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/963c25d2a13c11ddc7b9a3d4810cdff91e516299/

Edited pages:

v19.2/build-a-java-app-with-cockroachdb-hibernate.md

cockroach-teamcity · 2019-10-31T20:29:02Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/1bf4846b4784176634c6162e76cf244c7fe46659/

Edited pages:

v19.2/build-a-java-app-with-cockroachdb-hibernate.md

rmloveland · 2019-10-31T20:45:41Z

Hey @rafiss I think the core of the retry loop is working well based on my local testing, but I feel like this code needs updates that would make it more idiomatic for real Hibernate users. PTAL and let me know what you think.

(FYI I'm only updating the "insecure" example in this PR to keep it small while we fix the code and content - once we're happyish with it I'll also update the "secure" example and descriptions.)

rmloveland · 2019-11-11T18:47:35Z

@rafiss, ping. IDK if this can be ready for the release tomorrow but I'd love to get it in this week if at all possible!

rafiss

Sorry for the delay on the review! happy to sit next to you and show you my comments, or if it would help, i could even make some code changes myself, and it might be quicker. just let me know what works best for you! :)

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss and @rmloveland)

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 57 at r1 (raw file):

    }

    private static Random rand = new Random();

should be private static final

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 62 at r1 (raw file):

    static Function addAccounts(Session s1) throws JDBCException{
        Function f = (Function<Session, Integer>) s -> {
            Integer rv = 0;

replace these lines with

   private static Function<Session, Long> addAccounts() throws JDBCException{
        Function<Session, Long> f = s -> {
            long rv = 0;

to explain the changes:

function should be private
no need for the Session s1 parameter
the <...> syntax should be used in the return type
and also in the first line
this avoids the need for the cast
use long instead of Integer

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 70 at r1 (raw file):

                System.out.printf("APP: addAccounts() --> %d\n", rv);
            } catch (JDBCException e) {
                rv = -1;

remove rv = -1

that doesn't get used anywhere

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 80 at r1 (raw file):

    static Function transferFunds(long fromId, long toId, long amount) throws JDBCException{
        Function f = (Function<Session, Long>) s -> {
            long rv = 0;

replace these with

    private static Function<Session, Long> transferFunds(long fromId, long toId, long amount) throws JDBCException{
        Function<Session, Long> f = s -> {
            long rv = 0;

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 93 at r1 (raw file):

                }
            } catch (JDBCException e) {
                rv = -1;

remove rv = -1

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 104 at r1 (raw file):

    static Function testRetryLogic() throws JDBCException {
        Function f = (Function<Session, Integer>) s -> {
            Integer rv = -1;

replace with

    private static Function<Session, Long> forceRetryLogic() throws JDBCException {
        Function<Session, Long> f = s -> {
            long rv = -1;

(note that i renamed the function name. having "test" in the name is misleading)

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 122 at r1 (raw file):

Quoted 5 lines of code…

    static Function getAccountBalance(long id) throws JDBCException{
        Function f = (Function<Session, Long>) s -> {
            long balance;
            try {
                Account account = (Account) s.get(Account.class, id);

replace these with

    private static Function<Session, Long> getAccountBalance(long id) throws JDBCException{
        Function<Session, Long> f = s -> {
            long balance;
            try {
                Account account = s.get(Account.class, id);

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 138 at r1 (raw file):

    private static Number runTransaction(Session session, Function<Session, ? extends Number> fn) {
        Number rv = 0;

replace this with

    private static long runTransaction(Session session, Function<Session, Long> fn) {
        long rv = 0;

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 154 at r1 (raw file):

Quoted 4 lines of code…

            if (retryCount == MAX_RETRY_COUNT) {
                String err = String.format("hit max of %s retries, aborting", MAX_RETRY_COUNT);
                throw new RuntimeException(err);
            }

this block of code will never be reached. to fix this, i would rename retryCount to attemptCount, and increment attemptCount as the first thing in the loop. (and remove the incrementing in the catch block below)

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 179 at r1 (raw file):

                    System.out.printf("APP: retryable exception occurred:\n    sql state = [%s]\n    message = [%s]\n    retry counter = %s\n", e.getSQLState(), e.getMessage(), retryCount);
                    System.out.printf("APP: ROLLBACK;\n");
                    retryCount++;

remove this retryCount++ line if you take my suggestion above

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 190 at r1 (raw file):

                    rv = -1;
                } else {
                    rv = -1;

no need for this rv = -1 line

_includes/v19.2/app/insecure/hibernate-basic-sample/Sample.java, line 213 at r1 (raw file):

                Number fromBalance = runTransaction(session, getAccountBalance(fromAccountId));
                Number toBalance = runTransaction(session, getAccountBalance(toAccountId));
                if (fromBalance.intValue() != -1 && toBalance.intValue() != -1) {

remove all the intValue() calls everywhere in the code. it's not necessary and is kind of unconventional

rmloveland

i could even make some code changes myself, and it might be quicker

Honestly if you really don't mind, I agree that would be faster than me fumbling through this and fixing what I would inevitably break

Feel free to force push away :-)

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rmloveland)

rafiss

Got it! I have force pushed my changes, so this is good to merge from my end whenever you have everything else ready

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rmloveland)

cockroach-teamcity · 2019-11-21T21:07:01Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/f96be7a2c1b1de189aaecff57350733ffdf189e5/

Edited pages:

v19.2/build-a-java-app-with-cockroachdb-hibernate.md

rmloveland · 2019-11-21T21:24:50Z

Thanks @rafiss !!! I really appreciate your help.

I will now start the following tasks which will generate a bit of noise, but none of them should require any effort on your part; they're mostly editing-related tasks:

Update the "secure" version with this code and these docs
Copy these changes to the 19.1 and the 20.1 docs as well
Kick off the docs team review process so we can merge this :-)

cockroach-teamcity · 2019-11-22T17:34:30Z

Online preview: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/08895baec5e8a2ba385602fd5da9182c8d1508cc/

Edited pages:

rmloveland · 2019-11-22T17:52:48Z

@ericharmeling this one is a little complicated logistically - would appreciate your careful review. The actual updates are pretty much the same for 19.1, 19.2, and 20.1. My main concern is ensuring that I did not insert any subtle errors to the docs pages when copying all the different files around, and rebasing my local branch atop the many changes to master that have occurred since this PR was opened. There is also some complexity around the "Secure" vs. "Insecure" sections of each page, and ensuring that links point to the correct versions of the Hibernate app tarballs. (Really looking forward to avoiding a lot of this pain from now on as soon as #5885 gets done!)

I'll be OOO next week but will jump on any review comments first thing the Monday AM after that!

rmloveland · 2019-12-05T15:58:36Z

@ericharmeling ping :-)

ericharmeling

Thanks for the context, @rmloveland.

All of the example code is identical across versions with the following exceptions:

No changes to _includes/v19.1/app/insecure/hibernate-basic-sample are part of this PR. Are we not updating the insecure example for 19.1?
_includes/v20.1/app/insecure/hibernate-basic-sample/hibernate.cfg.xml includes a secure connection string. Shouldn't this be identical to _includes/v19.2/app/insecure/hibernate-basic-sample/hibernate.cfg.xml?

Also, unless I am missing something, there are a lot of "insecure" links and commands in secure "Build an App" doc sections (mainly in 19.1 and 20.1).

Reviewed 1 of 4 files at r1, 16 of 17 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ericharmeling and @rmloveland)

_includes/v20.1/app/insecure/hibernate-basic-sample/hibernate.cfg.xml, line 11 at r2 (raw file):

true&sslmode=require&sslrootcert=certs/ca.crt&sslkey=certs/client.maxroach.key.pk8&sslcert=certs/client.maxroach.crt]]

Isn't this an insecure example?

v19.1/build-a-java-app-with-cockroachdb-hibernate.md, line 88 at r2 (raw file):

1. Download and extract [hibernate-basic-sample.tgz](https://github.com/cockroachdb/docs/raw/master/_includes/{{ page.version.version }}/app/insecure/hibernate-basic-sample/hibernate-basic-sample.tgz). The settings in [`hibernate.cfg.xml`](https://github.com/cockroachdb/docs/raw/master/_includes/{{ page.version.version }}/app/insecure/hibernate-basic-sample/hibernate.cfg.xml) specify how to connect to the database.
2. Compile and run the code using [`build.gradle`](https://github.com/cockroachdb/docs/raw/master/_includes/{{ page.version.version }}/app/insecure/hibernate-basic-sample/build.gradle), which will also download the dependencies.

These are all "insecure" files, and we are still in the secure section of the doc.

v19.1/build-a-java-app-with-cockroachdb-hibernate.md, line 96 at r2 (raw file):

The contents of [`Sample.java`](https://raw.githubusercontent.com/cockroachdb/docs/master/_includes/{{page.version.version}}/app/insecure/hibernate-basic-sample/Sample.java):