Does Tx.WriteTo need to have Tx open for entire duration of io.CopyN?

[andreyvit]

Tx.WriteTo is fundamentally a long-running operation, it needs to read and write the entire size of the database. If, meanwhile, a write transaction needs the database to grow, everything gets stuck until the backup (WriteTo) completes. This is unfortunate.

After writing the meta pages, why does Tx.WriteTo need to keep holding the transaction open? Can we make a different method that simply returns the open file and size, and allows the caller to proceed with io.CopyN, while releasing the read transaction?

Any pitfalls here? I guess there's a concern about some required pages being overwritten, resulting in an inconsistent state? If yes, is there another way to avoid WriteTo blocking mmap resizing, perhaps (again, in a new method) by taking over the Tx entirely, and marking it as not blocking mmaps?

[ahrtr]

Since it's just a question, so I converted it into a discussion topic.

After writing the meta pages, why does Tx.WriteTo need to keep holding the transaction open?

It guarantees repeatable read. If you close the readonly transaction, then there will be no guarantee on repeatable read. No transaction, no guarantees.

Tx.WriteTo is fundamentally a long-running operation, it needs to read and write the entire size of the database. If, meanwhile, a write transaction needs the database to grow, everything gets stuck until the backup (WriteTo) completes. This is unfortunate.

Yes, it's correct based on current design & implementation. Let me know if you have any thoughts to improve this.

[andreyvit]

I'm not actually sure how GitHub discussions work, so gonna ping you here in case you're not getting the notifications from another response. Sorry if this results in duplicate ones. 🙌

[andreyvit]

@ahrtr Yes, backups can be improved: WriteTo isn't using the mmap'ed data during the long-running operation, so there's no need to block further mmaps. All it needs the Tx for is making sure that old pages don't get reused, right?

So while WriteTo is running, we don't need to have db.mmaplock RLock'ed.

Now, achieving that can be tricky, and, from what I can see, will require API contract changes either way:

a) We can just do db.mmaplock.RUnock at the start of WriteTo and then re-lock upon exit (and flush/re-read anything in Tx that depends on mmap'ed data). This won't change API signatures, but the contract still changes: all []bytes and (AFAIK) cursors are now invalidated by WriteTo.

b) We can add DB.WriteTo which opens its own transaction and does early mmaplock.RUnlock. This transaction would be unsafe to use for anything else by the client code, so shouldn't escape WriteTo call. I could see clients wanting to read some state consistently with a backup, though, so perhaps WriteTo inheriting an external transaction but closing it / marking it unusable is a better API.

c) add Tx.SafeWriteTo which is like Tx.WriteTo but documented to invalidate byte slices and cursors, i.e. (a) in a separate method

d) add a database flag that enables the new behavior of Tx.WriteTo described in (a)

e) change Tx.WriteTo to the new behavior described in (a), but add a database flag that restores the old one in case anyone relies on it

Or maybe we even decide that it's very unlikely that anyone in the wild carries byte slices or cursors over the call to WriteTo, and just do (a).

I would be happy to contribute implementation of either approach.

[andreyvit]

@ahrtr Could I possibly ping you on this? I'd love to do an implementation, but need to agree with you on an approach that you'll be willing to accept as PR.

[andreyvit]

PS: I personally think we can go for an options flag that enables the new behavior of Tx.WriteTo. The only concern is how to get from that state to a future where that behavior is the default. Perhaps the flag should be a *bool, and we can default to false for now, but to true in a few releases.

[ahrtr]

Firstly, I assume that you understand how bbolt works. Refer to https://github.com/ahrtr/etcd-issues/blob/master/docs/cncf_storage_tag_etcd.md#storage-boltdb-feature

Let me try to summarize this.

What Is the Problem?

Tx.WriteTo may block a RW transaction for a long time.

It may take a long time for Tx.WriteTo to complete. If a RW transaction needs to increase the db size during Tx.WriteTo running, accordingly need to remap the db file, then it will get blocked by the readonly transaction created by Tx.WriteTo.

What do we expect?

Tx.WriteTo does't block RW transaction

Potential solution

(preferred) Increase the db size before executing Tx.WriteTo

In current design/implementation, a RW transaction won't be blocked by any readonly transaction if the RW transaction doesn't need to increase the db size (remap). But if there isn't enough free page, then the RW needs to remap, accordingly will be blocked if there is any readonly transaction.

So a workaround is to increase the db size to allocate more free page beforehand, before it executes Tx.WriteTo.

The good side: this solution is simple, accordingly low risk.

bad size: Sometimes it may a little hard to evaluate the proper size which needs to be increased. So we may waste some disk space, and need more additional operation to reclaim the free pages.

RW transaction doesn't reuse free page during Tx.WriteTo

Firstly we don't create a readonly transaction when executing Tx.WriteTo, so it won't block the RW transaction. We just set a flag before executing Tx.WriteTo, and unset it after it's completed.

Secondly, in order to ensure the RW transaction won't corrupt the readonly transaction (Tx.WriteTo), it doesn't reuse the free page if the flag mentioned above is set, and always try to allocate new pages when needed.

The good side: it only allocates required space.
The bad side: It's a little complicated, and accordingly high risk.