Skip to content
This repository has been archived by the owner on Mar 9, 2019. It is now read-only.

Copy/CopyFile does not guarantee consistent meta page #84

Closed
tv42 opened this issue Mar 23, 2014 · 3 comments · Fixed by #92
Closed

Copy/CopyFile does not guarantee consistent meta page #84

tv42 opened this issue Mar 23, 2014 · 3 comments · Fixed by #92

Comments

@tv42
Copy link
Contributor

tv42 commented Mar 23, 2014

There's nothing synchronizing commit completion and Copy start. It currently relies on a1) os.File.Read not returning short reads and a2) io.Copy buffer being >= meta page size and a3) io.Copy buffer size not ending in middle of second meta page and AND b) the passed in writer not implementing io.ReaderFrom with a small buffer.

LMDB seems to do this by temporarily holding some extra lock, but I'm not quite familiar enough with the codebase to see what's going on.

Perhaps DB.Copy should just ask the tx.meta to serialize itself? Synthetize the content for the meta pages, copy the rest.

Also, to minimize error sources, it probably doesn't need to open a new fd for the read.. though I guess that gives you an independent read offset. This code achieves the same effect:

// Make an io.Reader out of an io.ReaderAt
func NewCursorReader(rat io.ReaderAt) io.Reader {
        // vastly overestimates the limit, but that shouldn't hurt it;
        // .Read() will just pass on the underlying ReaderAt's EOF.
        return io.NewSectionReader(rat, 0, math.MaxInt64)
}
@tv42
Copy link
Contributor Author

tv42 commented Mar 23, 2014

This seems to be the gist of the LMDB logic:

    if (env->me_txns) {
        /* We must start the actual read txn after blocking writers */
        mdb_txn_reset0(txn, "reset-stage1");

        /* Temporarily block writers until we snapshot the meta pages */
        LOCK_MUTEX_W(env);

        rc = mdb_txn_renew0(txn);
        if (rc) {
            UNLOCK_MUTEX_W(env);
            goto leave;
        }
    }

So, if there are (write) transactions open, prevent new ones, then re-read meta page. I think.

@benbjohnson
Copy link
Member

@tv42 Thanks again for being thorough. :)

I'll get db.Copy() cleaned up and add a meta lock.

@benbjohnson
Copy link
Member

I didn't know about io.NewSectionReader. Thanks! 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants