Re: [PATCH] Support aborting the atomic context
Floris Bruynooghe writes: > Since it is possible to use an atomic context to abort a number of > changes support this usage. Because the only way to actually abort > the transaction is to close the database this must also do so. merged to release and master, with a note about the anticipated update path. d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Support aborting the atomic context
On Mon 15 Jun 2020 at 07:35 -0300, David Bremner wrote: > Floris Bruynooghe writes: > >> This is an implementation of what was suggested in >> id:87v9k96xtl@powell.devork.be It closes the database as that is >> the only safe way to do this afaik. >> >> Currently when the database is closed there are still a bunch of >> operations which can result in segfaults. I realise that this is perhaps not a very helpful comment. I'll see if I can narrow this down into a proper bug report. >> Yet the API also promises >> that some operations are still valid, basically those which only >> access already previously retrieved information. It would be nice to >> find a good solution for this in the python bindings, but I don't yet >> know what this would be. > > There is a Xapian method WritableDatabase::cancel_transaction(), but it > is not currently exposed by notmuch. I think it could be added, but > getting the testing working right is not something I want to tackle at > this point in the release cycle. I guess this should be upwardly > compatible, as all code correct for your current implementation should > still work if we replace notmuch_database_close with > notmuch_cancel_atomic. Thoughts? I agree with your reasoning here that such a change would be upwardly compatible. Though I can think of some really convoluted scenarios where this could be false, e.g. after aborting a transaction code could rely on handling NOTMUCH_STATUS_XAPIAN_EXCEPTION it gets from a the closed database. This seems so much a corner case though, that I'd be willing to ignore it. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] Support aborting the atomic context
Floris Bruynooghe writes: > This is an implementation of what was suggested in > id:87v9k96xtl@powell.devork.be It closes the database as that is > the only safe way to do this afaik. > > Currently when the database is closed there are still a bunch of > operations which can result in segfaults. Yet the API also promises > that some operations are still valid, basically those which only > access already previously retrieved information. It would be nice to > find a good solution for this in the python bindings, but I don't yet > know what this would be. There is a Xapian method WritableDatabase::cancel_transaction(), but it is not currently exposed by notmuch. I think it could be added, but getting the testing working right is not something I want to tackle at this point in the release cycle. I guess this should be upwardly compatible, as all code correct for your current implementation should still work if we replace notmuch_database_close with notmuch_cancel_atomic. Thoughts? d ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Support aborting the atomic context
Since it is possible to use an atomic context to abort a number of changes support this usage. Because the only way to actually abort the transaction is to close the database this must also do so. --- bindings/python-cffi/notmuch2/_database.py | 16 +++- bindings/python-cffi/tests/test_database.py | 5 + 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bindings/python-cffi/notmuch2/_database.py b/bindings/python-cffi/notmuch2/_database.py index 95f59ca0..c851f0a5 100644 --- a/bindings/python-cffi/notmuch2/_database.py +++ b/bindings/python-cffi/notmuch2/_database.py @@ -641,6 +641,7 @@ class AtomicContext: def __init__(self, db, ptr_name): self._db = db self._ptr = lambda: getattr(db, ptr_name) +self._exit_fn = lambda: None def __del__(self): self._destroy() @@ -656,13 +657,17 @@ class AtomicContext: ret = capi.lib.notmuch_database_begin_atomic(self._ptr()) if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: raise errors.NotmuchError(ret) +self._exit_fn = self._end_atomic return self -def __exit__(self, exc_type, exc_value, traceback): +def _end_atomic(self): ret = capi.lib.notmuch_database_end_atomic(self._ptr()) if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: raise errors.NotmuchError(ret) +def __exit__(self, exc_type, exc_value, traceback): +self._exit_fn() + def force_end(self): """Force ending the atomic section. @@ -681,6 +686,15 @@ class AtomicContext: if ret != capi.lib.NOTMUCH_STATUS_SUCCESS: raise errors.NotmuchError(ret) +def abort(self): +"""Abort the transaction. + +Aborting a transaction will not commit any of the changes, but +will also implicitly close the database. +""" +self._exit_fn = lambda: None +self._db.close() + @functools.total_ordering class DbRevision: diff --git a/bindings/python-cffi/tests/test_database.py b/bindings/python-cffi/tests/test_database.py index e3a8344d..aa2cbdc7 100644 --- a/bindings/python-cffi/tests/test_database.py +++ b/bindings/python-cffi/tests/test_database.py @@ -127,6 +127,11 @@ class TestAtomic: with pytest.raises(errors.UnbalancedAtomicError): ctx.force_end() +def test_abort(self, db): +with db.atomic() as txn: +txn.abort() +assert db.closed + class TestRevision: -- 2.27.0 ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] Support aborting the atomic context
This is an implementation of what was suggested in id:87v9k96xtl@powell.devork.be It closes the database as that is the only safe way to do this afaik. Currently when the database is closed there are still a bunch of operations which can result in segfaults. Yet the API also promises that some operations are still valid, basically those which only access already previously retrieved information. It would be nice to find a good solution for this in the python bindings, but I don't yet know what this would be. ___ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch
