[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: (ITS#7377) Poor libmdb error handling



h.b.furuseth@usit.uio.no wrote:
> Changes from a failed liblmdb operation can be visible.  Sometimes
> the resulting DB will also be inconsistent.  The operation should
> invalidate the affected transaction/cursors or revert the change.

The transaction should be invalidated. Nothing more can be done once any error 
occurs in a transaction.

> At least I hope it's feasible to invalidate so the user cannot see
> surprising results, instead of just documenting "don't do that".
>
> Related issues:
>
> put(MDB_MULTIPLE) can write some of the provided items and then
> fail, but be able to keep the txn usable.  It should invalidate the
> txn anyway, or document that the caller must check for e.g. return
> value MDB_PARTIAL_SUCCESS.  Maybe invalidate unless the the caller
> requests support for partial success. put() would update the input
> params to indicate which items remain unwritten.

No, "partial success" is not allowed. A transaction is atomic, all or nothing.

> A failed txn could still grow the txn size, unless MDB "un-touches"
> pages which memcmp says have not changed, rewinds me_pglast and
> me_pghead, etc.  Which seems a lot of work, likely not worth the
> trouble.

pghead/pglast should not persist into the environment until a successful 
commit. Until then any error should just discard them.

>
> I think cursors need a C_INVALID flag, so future ops cannot give a
> surprising success result instead of failing: get(MDB_NEXT/MDB_PREV)
> take ~C_INITIALIZED to mean (re)start from the beginning/end.

Good idea.

> Don't know how often reverting is a relevant option. Rearranging
> code can help, but maybe not much.  E.g. the mdb_del() in mdb_drop()
> can do some harmless touches and then fail cleanly - but mdb_drop()
> must invalidate the txn anyway since mdb_drop0() has deleted pages.
> Unless it does extra work - remember mt_free_pgs[0] before and after
> mdb_drop0(), then delete from mt_free_pgs[] the pages added by
> drop0.  Or drop0 could be done last since it can fail cleanly just
> by resetting mt_free_pgs[0]. But I don't know if del(MDB_MULTIPLE)
> can also do that if the DB used a subpage. drop0 can't examine the
> subpage after it got deleted.
>
Reverting is irrelevant. Once a txn is aborted no changes are visible.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/