1. 31 Mar, 2020 1 commit
    • Kirill Smelkov's avatar
      FileStorage: Save committed transaction to disk even if changed data is empty · fdf9e7a2
      Kirill Smelkov authored
      [ This is ZODB4 backport of commit bb9bf539
        (https://github.com/zopefoundation/ZODB/pull/298) ]
      
      ZODB tries to avoid saving empty transactions to storage on
      `transaction.commit()`. The way it works is: if no objects were changed
      during ongoing transaction, ZODB.Connection does not join current
      TransactionManager, and transaction.commit() performs two-phase commit
      protocol only on joined DataManagers. In other words if no objects were
      changed, no tpc_*() methods are called at all on ZODB.Connection at
      transaction.commit() time.
      
      This way application servers like Zope/ZServer/ERP5/... can have
      something as
      
          try:
              # process incoming request
              transaction.commit()    # processed ok
          except:
              transaction.abort()
              # problem: log + reraise
      
      in top-level code to process requests without creating many on-disk
      transactions with empty data changes just because read-only requests
      were served.
      
      Everything is working as intended.
      
      However at storage level, FileStorage currently also checks whether
      transaction that is being committed also comes with empty data changes,
      and _skips_ saving transaction into disk *at all* for such cases, even
      if it has been explicitly told to commit the transaction via two-phase
      commit protocol calls done at storage level.
      
      This creates the situation, where contrary to promise in
      ZODB/interfaces.py(*), after successful tpc_begin/tpc_vote/tpc_finish()
      calls made at storage level, transaction is _not_ made permanent,
      despite tid of "committed" transaction being returned to caller. In other
      words FileStorage, when asked to commit a transaction, even if one with
      empty data changes, reports "ok" and gives transaction ID to the caller,
      without creating corresponding transaction record on disk.
      
      This behaviour is
      
      a) redundant to application-level avoidance to create empty transaction
         on storage described in the beginning, and
      
      b) creates problems:
      
      The first problem is that application that works at storage-level might
      be interested in persisting transaction, even with empty changes to
      data, just because it wants to save the metadata similarly to e.g.
      `git commit --allow-empty`.
      
      The other problem is that an application view and data in database
      become inconsistent: an application is told that a transaction was
      created with corresponding transaction ID, but if the storage is
      actually inspected, e.g. by iteration, the transaction is not there.
      This, in particular, can create problems if TID of committed transaction
      is reported elsewhere and that second database client does not find the
      transaction it was told should exist.
      
      I hit this particular problem with wendelin.core. In wendelin.core,
      there is custom virtual memory layer that keeps memory in sync with
      data in ZODB. At commit time, the memory is inspected for being dirtied,
      and if a page was changed, virtual memory layer joins current
      transaction _and_ forces corresponding ZODB.Connection - via which it
      will be saving data into ZODB objects - to join the transaction too,
      because it would be too late to join ZODB.Connection after 2PC process
      has begun(+). One of the format in which data are saved tries to
      optimize disk space usage, and it actually might happen, that even if
      data in RAM were dirtied, the data itself stayed the same and so nothing
      should be saved into ZODB. However ZODB.Connection is already joined
      into transaction and it is hard not to join it because joining a
      DataManager when the 2PC is already ongoing does not work.
      
      This used to work ok with wendelin.core 1, but with wendelin.core 2 -
      where separate virtual filesystem is also connected to the database to
      provide base layer for arrays mappings - this creates problem, because
      when wcfs (the filesystem) is told to synchronize to view the database
      @tid of committed transaction, it can wait forever waiting for that, or
      later, transaction to appear on disk in the database, creating
      application-level deadlock.
      
      I agree that some more effort might be made at wendelin.core side to
      avoid committing transactions with empty data at storage level.
      
      However the most clean way to fix this problem in my view is to fix
      FileStorage itself, because if at storage level it was asked to commit
      something, it should not silently skip doing so and dropping even non-empty
      metadata + returning ok and committed transaction ID to the caller.
      
      As described in the beginning this should not create problems for
      application-level ZODB users, while at storage-level the implementation
      is now consistently matching interface and common sense.
      
      ----
      
      (*) tpc_finish: Finish the transaction, making any transaction changes permanent.
          Changes must be made permanent at this point.
          ...
      
          https://github.com/zopefoundation/ZODB/blob/5.5.1-35-gb5895a5c2/src/ZODB/interfaces.py#L828-L831
      
      (+) https://lab.nexedi.com/kirr/wendelin.core/blob/9ff5ed32/bigfile/file_zodb.py#L788-822
      fdf9e7a2
  2. 30 Apr, 2018 1 commit
  3. 08 Apr, 2017 2 commits
  4. 03 Apr, 2017 1 commit
    • Kirill Smelkov's avatar
      FileStorage: Report problem on read-only open of non-existent file · 56c96a11
      Kirill Smelkov authored
      ... instead of silently creating empty database on such opens.
      
      Use-case for this are utilities like e.g. zodbdump and zodbcmp which
      expect such storage opens to fail so that the tool can know there is no
      such storage and report it to user.
      
      In contrast current state is: read-only opens get created-on-the-fly
      empty storage with no content, but which can be iterated over without
      getting any error.
      
      This way e.g. `zodbdump non-existent.fs` produces empty output _and_
      exit code 0 which is not what caller expects.
      
      (cherry picked from commit 30bbabf1)
      56c96a11
  5. 06 Feb, 2017 1 commit
  6. 02 Feb, 2017 5 commits
  7. 27 Nov, 2016 2 commits
  8. 25 Nov, 2016 1 commit
  9. 27 Sep, 2016 2 commits
  10. 12 Sep, 2016 2 commits
  11. 09 Sep, 2016 1 commit
  12. 21 Aug, 2016 2 commits
  13. 04 Aug, 2016 1 commit
  14. 27 Jul, 2016 2 commits
  15. 26 Jul, 2016 1 commit
  16. 13 Jul, 2016 1 commit
  17. 12 Jul, 2016 8 commits
    • Julien Muchembled's avatar
      fstail: print the txn offset and header size, instead of only the data offset · 3807ace8
      Julien Muchembled authored
      Before:
      
          2016-07-01 09:41:50.416574: hash=d7101c5ee7b8e412d7b6d54873204421e09b7f34
          user='' description='' length=1629 offset=58990284
      
      After:
      
          2016-07-01 09:41:50.416574: hash=d7101c5ee7b8e412d7b6d54873204421e09b7f34
          user='' description='' length=1629 offset=58990261 (+23)
      
      The structure of a FileStorage DB is such that it's easy to revert the last
      transactions, by truncating the file at the right offset. With the above
      change, `fstail` can now be used to get this offset.
      
      In the above example:
      
          truncate -s 58990261 Data.fs
      
      would delete the transaction and all those after.
      3807ace8
    • Jim Fulton's avatar
      Merge pull request #89 from zopefoundation/undo-refactor · 75bae1a6
      Jim Fulton authored
      Refactored FileStorage transactional undo
      75bae1a6
    • Jim Fulton's avatar
      removed out of date comment · b563487e
      Jim Fulton authored
      b563487e
    • Jim Fulton's avatar
      Merge pull request #86 from NextThought/handle-serials4 · e080bdcc
      Jim Fulton authored
      Fix handle_all_serials for the new and old protocols.
      e080bdcc
    • Jason Madden's avatar
      Update comment. [skip ci] · c13649da
      Jason Madden authored
      c13649da
    • Jim Fulton's avatar
      changes · d64a0cbf
      Jim Fulton authored
      d64a0cbf
    • Jim Fulton's avatar
      Refactored FileStorage transactional undo · d717a685
      Jim Fulton authored
      As part of a project to provide object-level commit locks for ZEO, I'm
      refactiring FileStorage to maintain transaction-specific data in
      Tranaction.data.  This involved undo.  In trying to figure this out, I
      found:
      
      - A bug in _undoDataInfo, which I verified with some tests and
      
      - _transactionalUndoRecord was maddeningly difficult to reason about
        (and thus change).
      
      I was concerned less by the bug than my inability to know whether a
      change to the code would be correct.
      
      So I refactored the code, mainly transactionalUndoRecord, to make the
      code easier to understand, fixing some logic errors (I'm pretty sure)
      along the way.  This included lots of comments. (Comments are much
      easier to compose when you're working out logic you didn't
      understand.)
      
      In addition to makeing the code cleaner, it allows undo to be handled
      in cases that weren't handled before.
      d717a685
    • Jim Fulton's avatar
      Long lines. Grrrr. · a6c1713d
      Jim Fulton authored
      a6c1713d
  18. 09 Jul, 2016 1 commit
  19. 08 Jul, 2016 1 commit
  20. 05 Jul, 2016 3 commits
  21. 04 Jul, 2016 1 commit