Merge pull request #166 from yetanalytics/txn_update

Make changes for certain Hikari config values

Makefile

@@ -53,15 +53,11 @@ persistent: resources/public/admin
 sqlite: resources/public/admin
 	clojure -X:db-sqlite lrsql.sqlite.main/run-test-sqlite
 
-# NOTE: Requires a running Postgres instance where:
-# - user is lrsql_user
-# - password is swordfish
-# - db name is lrsql_pg
-# - schema is lrsql
-postgres: resources/public/admin
+postgres: resources/public/admin # Requires a running Postgres instance
 	clojure -X:db-postgres lrsql.postgres.main/run-test-postgres
 
-# NOTE: Requires a running lrsql instance!
+# Bench - requires a running lrsql instance
+
 bench:
 	clojure -M:bench -m lrsql.bench \
 	    -e http://0.0.0.0:8080/xapi/statements \

doc/env_vars.md

@@ -23,7 +23,7 @@ All configuration variables can either be set directly via the command line as e
 
 #### HikariCP Properties
 
-The following environment variables are aliases for [HikariCP properties](https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby). All of these variables (except for `poolName`) have default values that are already set. Note that H2 and SQLite use different defaults for `poolMinimumIdle` and `poolMaximumSize` due to issues with multi-threading with those DBMSs. All temporal values are in milliseconds.
+The following environment variables are aliases for [HikariCP properties](https://github.com/brettwooldridge/HikariCP#gear-configuration-knobs-baby). All of these variables (except for `poolName`) have default values that are already set, which may vary depending on the DBMS. All temporal values are in milliseconds.
 
 | Env Var | Config | Default | Valid Values |
 | --- | --- | --- | --- |
@@ -38,13 +38,17 @@ The following environment variables are aliases for [HikariCP properties](https:
 | `LRSQL_POOL_MAXIMUM_SIZE` | `poolMaximumSize` | `1`* or `10`** | `≥ 1` |
 | `LRSQL_POOL_ISOLATE_INTERNAL_QUERIES` | `poolIsolateInternalQueries` | `false` | `true`/`false` |
 | `LRSQL_POOL_LEAK_DETECTION_THRESHOLD` | `poolLeakDetectionThreshold` | `0`† | `≥ 2000` or `0` |
-| `LRSQL_POOL_TRANSACTION_ISOLATION` | `poolTransactionIsolation` | Not set | [JDBC Connection constant](https://docs.oracle.com/javase/7/docs/api/java/sql/Connection.html) (e.g. `TRANSACTION_SERIALIZABLE`) |
+| `LRSQL_POOL_TRANSACTION_ISOLATION` | `poolTransactionIsolation` | Not set | [JDBC Connection constant](https://docs.oracle.com/en/java/javase/11/docs/api/java.sql/java/sql/Connection.html) (e.g. `TRANSACTION_SERIALIZABLE`) |
 | `LRSQL_POOL_NAME` | `poolName` | Not set | Any string |
 
 \* H2/SQLite default.
 \*\* Postgres default.
 † The property is set to be disabled by default.
 
+*NOTE 1:* H2 and SQLite use different defaults for `poolMinimumIdle` and `poolMaximumSize` than Postgres due to issues with multi-threading with those DBMSs. Setting `poolMaximumSize` to values other than `1` will potentially cause exceptions when running concurrent operations.
+
+*NOTE 2:* None of the DBMSs that SQL LRS currently supports allow for `TRANSACTION_NONE` as a `poolTransactionIsolation` value.
+
 #### Metric Reporting via JMX
 
 The following config var is to activate metrics reporting via JMX.

src/db/postgres/lrsql/postgres/record.clj

@@ -57,7 +57,10 @@
   (-txn-retry? [_ ex]
     ;; only retry PGExceptions with a specified phrase
     (and (instance? org.postgresql.util.PSQLException ex)
-         (includes? (.getMessage ex) "ERROR: deadlock detected")))
+         (let [msg (.getMessage ex)]
+           (or (includes? msg "ERROR: deadlock detected")
+               (includes? msg "ERROR: could not serialize access due to concurrent update")
+               (includes? msg "ERROR: could not serialize access due to read/write dependencies among transactions")))))
 
   bp/StatementBackend
   (-insert-statement! [_ tx input]

src/main/lrsql/spec/config.clj

@@ -67,15 +67,14 @@
         :enabled (s/and pos-int? (partial <= 2000))))
 
 (s/def ::pool-transaction-isolation
-  #{"TRANSACTION_NONE" ; This level exists but might cause problems
+  #{"TRANSACTION_NONE" ; This level exists but will cause problems
     "TRANSACTION_READ_UNCOMMITTED"
     "TRANSACTION_READ_COMMITTED"
     "TRANSACTION_REPEATABLE_READ"
     "TRANSACTION_SERIALIZABLE"})
 
 (s/def ::connection
   (s/and (s/conformer u/remove-nil-vals)
-         (s/conformer u/remove-neg-vals)
          (s/keys :req-un [::database
                           ::pool-auto-commit
                           ::pool-keepalive-time

src/main/lrsql/system/lrs.clj

@@ -54,10 +54,15 @@
           config
           ;; Authority function
           auth-fn (make-authority-fn auth-tp)]
-      (init/init-backend! backend conn)
-      (init/insert-default-creds! backend conn uname pass api-key srt-key)
-      (log/info "Starting new LRS")
-      (assoc lrs :connection connection :authority-fn auth-fn)))
+      ;; Combine all init ops into a single txn, since the user would expect
+      ;; such actions to happen as a single unit. If init-backend! succeeds
+      ;; but insert-default-creds! fails, this would constitute a partial
+      ;; application of what the user wanted.
+      (jdbc/with-transaction [tx conn]
+        (init/init-backend! backend tx)
+        (init/insert-default-creds! backend tx uname pass api-key srt-key)
+        (log/info "Starting new LRS")
+        (assoc lrs :connection connection :authority-fn auth-fn))))
   (stop
     [lrs]
     (log/info "Stopping LRS...")

src/main/lrsql/util/concurrency.clj

@@ -17,7 +17,7 @@
           :opt-un [::j-range ::initial]))
 
 (s/fdef backoff-ms
-  :args (s/cat :attempt pos-int?
+  :args (s/cat :attempt nat-int?
                :opts backoff-opts-spec)
   :ret (s/nilable nat-int?))