| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead. |
Sorry, something went wrong.
Sorry, something went wrong.
…ce to module links not found
…m.dumb for consistency with dbm.gnu. Also updated documentations and tests to reflect the change
There was a problem hiding this comment.
Very good code.
However, there are few issues.
dbm.dumb does not support concurrent writing, but it supports limited concurrent access with a single writer and multiple readers. Readers may not see new data, and can read deleted data (because they cache the index), but otherwise they do not conflict with writer. But when writer calls reorganize(), it breaks readers. So using this feature can cause new bugs.
I am not sure that we can to do something with this.
Other issue is that reorganize() is inherently not fail-safe. If something happens during the reorganization process, you may lose data.
Do gdbm_reorganize() and Sqlite VACUUM have the same issues or they provide some protection?
Sorry, something went wrong.
|
Good morning @serhiy-storchaka, Easy changesI have pushed changes to address the documentation and dbm.dumb comment remarks. Organize not being fail-safeOn the topic of reorganize() inherently being not fail-safe, I agree with you. It is my understanding that dbm.dumb does very little to be robust in general. Most notably, it relies on its __del__ method to write back, which may not always be called in case of trouble. This is why I chose this less-resource-intensive implementation over a more robust one. If we want to go for more robustness, any of these two approaches should work, although none have a 100% robustness guarantee:
Let me know how you think we should proceed. SQLITE and GDBM are designed to be much more robust and use the second technique to ensure failed reorganizations don't corrupt the database. Crucially, however, they handle a single binary file with both index and data, hence either everything is renamed atomically or nothing is. ConcurrencyMy apologies, but I would disagree dbm.dumb supports any concurrency at all... I totally understand what you mean but for me if the readers can access deleted keys and if they aren't notified when a value is updated for an existing key the behaviour during concurrency is undefined, not limitedly supported. Examples:
Of course this problem is well-known to any developer who looked at the source code for dbm and dbm.dumb. However, I found the documentation was a bit lackluster on these problems, so I have also updated it to warn users of these limitations (see the warning under dbm.dumb.open). One thing that was already well explained in the documentation, however:
Please let me know how you would like to proceed on this issue as well. In the meantime, have a great day! 😃 |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for your response. What is left:
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM. 👍
Thank you for your contribution, @Andrea-Oliveri. Please ping me next week if I don't merge this PR before then.
Sorry, something went wrong.
|
Thank you @serhiy-storchaka 😄 . Just FYI, since I had merge conflicts due to the whatsnew I have merged with main between you marking as approved and me reading the message. |
Sorry, something went wrong.
|
This is great; thanks for the proposal and implementation, @Andrea-Oliveri; thanks for managing this, @serhiy-storchaka. |
Sorry, something went wrong.
…mb and shelve (pythonGH-134028) They are similar to the same named method in dbm.gnu.
…mb and shelve (pythonGH-134028) They are similar to the same named method in dbm.gnu.
…mb and shelve (pythonGH-134028) They are similar to the same named method in dbm.gnu.
| Back | FazBrowse Home | New Git URL |
gh-134004:
Note: Unfortunately I did not feel comfortable changing the C code at this time, so I chose not to add an empty .reorganize() method to dbm.ndbm. This resulted in: