Dear Alex,
Thank you very much for taking the time to deal with this. I
personally don't have much of an opinion about how it should be
done---except that it sure seems like a good idea to make it
configurable, which ought to keep everybody happy, and prevents me
having to make a decision I am poorly-qualified to make.
Could you make this configurable? I would be happy to document it and
commit it if it were configurable (and if we can get to a proven green
state, which might be a little tricky right at the moment---but we can
save the patch for when we get it ironed out.)
On Sun, 2008-03-16 at 23:26 +0200, Alex Mizrahi wrote:
> LPP> It's a performance decision...
>
> as far as i know with MVCC higher isolation level does not cause performance
> penalties unless you really have conflicts.
> at least some postgresql experts said me this.
>
> LPP> go, but the rest of the code should try hard not to expose any
> LPP> avoidable errors if the user decides to switch to another isolation
> LPP> level...
>
> switching to another isolation level user gets concurrent conflicts. why
> should we try to detect them in our code when DB can handle them in
> centralized fashion?
> this sounds like an "ugly workaround" for me.
>
> LPP> In any case, can you send a patch for the change of the default
> LPP> isolation level, and probably a public convenience function/sc arg to
> LPP> change it?
>
> changing isolation level is dead simple, but it requires dealing with
> consequences. this was pretty hard for application i'm working with, so i
> had to make extensions like retry-cleanup-fn.
> and stuff is not configurable because i thought serializable would be good
> for everybody, but if you insist we should have "read committed" too, i'll
> make it for you.
> below is work-in-progress patch:
>
> --- old-elephant/src/db-postmodern/pm-transaction.lisp Wed Jan 16 23:07:43
> 2008
> +++ new-elephant/src/db-postmodern/pm-transaction.lisp Fri Mar 14 15:00:02
> 2008
> @@ -26,30 +26,70 @@
> (defun txn-cache-clear-value (bt key)
> (cache-clear-value *txn-value-cache* bt key))
>
> +(defun execute-transaction-one-try (sc txn-fn always-rollback)
> + (let (tran commited
> + (*txn-value-cache* (make-value-cache sc)))
> + (incf (tran-count-of sc))
> + (setf tran (controller-start-transaction sc))
> + (unwind-protect
> + (multiple-value-prog1
> + (funcall txn-fn)
> + (unless always-rollback ; automatically commit unless always-rollback
> is on
> + (controller-commit-transaction sc tran)
> + (setf commited t)))
> + (unless commited (controller-abort-transaction sc tran))
> + (decf (tran-count-of sc)))))
> +
> +(defmacro with-concurrency-errors-handler (&body body)
> + "execute body with a handler catching postgres concurrency errors
> + and invoking restart-transaction restart automatically"
> + `(handler-bind
> + ((cl-postgres:database-error
> + (lambda (c)
> + (let ((err-code (cl-postgres:database-error-code c)))
> + (when (or (string= err-code "40001") ; SERIALIZATION FAILURE
> + (string= err-code "40P01")); DEADLOCK DETECTED
> + (invoke-restart 'retry-transaction c))))))
> + ,@body))
> +
> (defmethod execute-transaction ((sc postmodern-store-controller) txn-fn
> - &key (always-rollback nil) &allow-other-keys)
> - ;; SQL doesn't support nested transaction
> + &key (always-rollback nil)
> + (retry-cleanup-fn nil)
> + (retries 10) &allow-other-keys)
> (with-postmodern-conn ((controller-connection-for-thread sc))
> (if (> (tran-count-of sc) 0)
> - (funcall txn-fn)
> - (let (tran
> - commited
> - (*txn-value-cache* (make-value-cache sc)))
> - (incf (tran-count-of sc))
> - (unwind-protect
> - (prog2
> - (setf tran (controller-start-transaction sc))
> - (funcall txn-fn) ;;this gets returned
> - (unless always-rollback ;;automatically commit unless always rollback
> - (controller-commit-transaction sc tran)
> - (setf commited t)))
> - (unless commited (controller-abort-transaction sc tran))
> - (decf (tran-count-of sc)))))))
> +
> + ;; SQL doesn't support nested transaction
> + ;; TODO: perhaps it's worth detecting abnormal exit here
> + ;; and abort parent transaction too.
> + (with-concurrency-errors-handler (funcall txn-fn))
> +
> + (loop named txn-retry-loop
> + ;; NB: it does (1+ retries) attempts, 1 try + retries.
> + for try from retries downto 0
> + do (block txn-block
> + (restart-bind ((retry-transaction
> + (lambda (&optional condition)
> + (when (and retry-cleanup-fn
> + (not (= try 0))) ; cleanup is skipped when we are exiting
> + (funcall retry-cleanup-fn condition sc))
> + (return-from txn-block))
> + :report-function (lambda (s) (princ "retry db-postmodern
> transaction" s)))
> + (abort-transaction
> + (lambda () (return-from txn-retry-loop))))
> + (with-concurrency-errors-handler
> + (return-from txn-retry-loop
> + (execute-transaction-one-try sc txn-fn always-rollback)))))
> + finally (error 'transaction-retry-count-exceeded
> + :format-control "Transaction exceeded the ~A retries limit"
> + :format-arguments (list retries)
> + :count retries)))))
> +
>
> (defmethod controller-start-transaction ((sc postmodern-store-controller)
> &key &allow-other-keys)
> (with-postmodern-conn ((controller-connection-for-thread sc))
> (let ((transaction (make-instance 'postmodern::transaction-handle)))
> - (postmodern:execute "BEGIN")
> + (postmodern:execute "BEGIN ISOLATION LEVEL SERIALIZABLE")
> transaction)))
>
> (defmethod controller-commit-transaction ((sc postmodern-store-controller)
> transaction &key &allow-other-keys)
>
>
>
>
>
>
> _______________________________________________
> elephant-devel site list
> elephant-devel@common-lisp.net
> http://common-lisp.net/mailman/listinfo/elephant-devel
_______________________________________________
elephant-devel site list
elephant-devel@common-lisp.net
http://common-lisp.net/mailman/listinfo/elephant-devel
