Hi @mattsains. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

@fuweid , would you be able to take a look at this PR?

Investigating the test-windows failure, I discovered that the db.allocate function can actually cause the file size to grow through the truncate in the mmap function. Should I change the allocate function to call grow if a request to allocate beyond the end of the file is received, or should I just duplicate the max size check in allocate?

specifically, this line here causes the file size to grow: https://github.com/etcd-io/bbolt/blob/main/db.go#L1168

@fuweid , I've added a test for making sure the db can open even if it's beyond the configured max size. I've also got all the builds to pass, so I think this is ready for a review and potentially approval

hi @fuweid , I am awaiting feedback from you for this PR

Thanks for the PR.

My thoughts,

• This PR might be affecting the existing db file, which already reaches the max size. So it's a breaking change.
• Also it updates multiple places, which complicates the implementation.
• It's hard to accurately control the max size; instead It should be a best-effort solution.
  • FYI. etcd implements similar max size (quota) controlling using flag --quota-backend-bytes , which is best effort. Also it won't lose data (return error after persisting the data)
  • Also there may be hundreds of etcd's transactions in each bbolt's transaction. etcd controls the db quota on etcd's transaction granularity instead of bbolt's transaction level.

Overall, I agree that it's a valid requirement. However, I don't feel safe to support it in bbolt. Instead, I'd suggest to add this feature in your application, and as a best-effort approach.

Could you explain what you mean by:

This PR might be affecting the existing db file, which already reaches the max size. So it's a breaking change.

The max file size does not take effect unless (1) you turn it on in the configuration, and (b) you write to a database in a way that causes it to grow

Also it updates multiple places, which complicates the implementation.

I am happy to change it to only one location, but it seems to me that there are many places in the code where the size of the file is unintentionally grown, for example when mmaping the file on windows

It's hard to accurately control the max size; instead It should be a best-effort solution.

What makes it hard? I think this PR does it, does it not? Am I missing some complexity?

I'd suggest to add this feature in your application, and as a best-effort approach.

I am really trying to make this maximum feature not be best effort, which is why I created this PR

Look forward to your comments, @ahrtr

@ahrtr hi there, I think this is ready for re-review

@ahrtr I've updated the PR based on your comments