Merge ecddf06 into e20d8b9

CHANGES.txt

@@ -18,8 +18,8 @@
 
 - zodbconvert: The ``--incremental`` option is supported with a
   FileStorage (or any storage that implements
-  ``IStorage.lastTransaction()``)
-  as a destination, not just RelStorages.
+  ``IStorage.lastTransaction()``) as a destination, not just
+  RelStorages.
 
 - zodbconvert: The ``--incremental`` option is supported with a
   RelStorage as a destination. See `PR 22`_. With contributions by
@@ -34,6 +34,10 @@
 
 .. _`PR 18`: https://github.com/zodb/relstorage/pull/18/
 
+- ``RelStorage.lastTransaction()`` is more consistent with FileStorage
+  and ClientStorage, returning a useful value in more cases
+  (specifically, after a connection is made but before any polls are
+  done or transactions committed).
 
 1.6.0b3 (2014-12-08)
 --------------------

relstorage/storage.py

@@ -963,7 +963,16 @@ def _abort(self):
     def lastTransaction(self):
         self._lock_acquire()
         try:
-            return self._ltid
+            if (self._ltid == z64
+                and self._prev_polled_tid is None
+                and self._load_transaction_open):
+                # We haven't committed *or* polled for transactions,
+                # so our MVCC state is "floating".
+                # Read directly from the database to get the latest value,
+                # but don't force a connection just to do that (just like ClientStorage).
+                return self._adapter.txncontrol.get_tid(self._load_cursor)
+
+            return max(self._ltid, p64(self._prev_polled_tid or 0))
         finally:
             self._lock_release()
 

relstorage/tests/test_zodbconvert.py

@@ -76,7 +76,6 @@ def test_dry_run(self):
         main(['', '--dry-run', self.cfgfile])
         self._check_value_of_x_in_dest(None)
 
-
     def test_incremental(self):
         x = 10
         self._write_value_for_x_in_src(x)
@@ -88,6 +87,10 @@ def test_incremental(self):
         main(['', '--incremental', self.cfgfile])
         self._check_value_of_x_in_dest(x)
 
+    def test_incremental_empty_src_dest(self):
+        # Should work and not raise a POSKeyError
+        main(['', '--incremental', self.cfgfile])
+        self._check_value_of_x_in_dest(None)
 
     def test_no_overwrite(self):
         db = self._create_src_db() # create the root object

relstorage/zodbconvert.py

@@ -105,33 +105,31 @@ def main(argv=sys.argv):
     log.info("Storages opened successfully.")
 
     if options.incremental:
-        if not hasattr(destination, '_adapter') and not hasattr(destination, 'lastTransaction'):
+        if not hasattr(destination, 'lastTransaction'):
             msg = ("Error: no API is known for determining the last committed "
                    "transaction of the destination storage. Aborting "
                    "conversion.")
             sys.exit(msg)
         if not storage_has_data(destination):
             log.warning("Destination empty, start conversion from the beginning.")
         else:
-            wrap_source = False
-            if hasattr(destination, '_adapter'):
-                # RelStorage. Note that we implement lastTransaction(), but
-                # only in-memory, local to the particular object. (We should probably
-                # change that?) So order matters.
-                destination.load(z64) # prime the connection
-                wrap_source = True # compensate for our bug
-                last_tid = destination._adapter.txncontrol.get_tid(
-                    destination._load_cursor)
-            else:
-                # IStorage, like FileStorage
-                last_tid_s = destination.lastTransaction()
-                last_tid = u64(last_tid_s)
+            # First, make sure the connection is fully connected. This
+            # matters for ClientStorage (connects async) and RelStorage
+            # (connects on demand).
+            # If we hadn't already checked that the destination has data,
+            # this would raise a POSKeyError.
+            destination.load(z64)
+            last_tid = destination.lastTransaction()
+            if isinstance(last_tid, bytes):
+                # FileStorage returns a byte string, everything else
+                # returns an int
+                last_tid = u64(last_tid)
 
             next_tid = p64(last_tid+1)
-            if wrap_source:
-                source = _DefaultStartStorageIteration(source, next_tid)
-            else:
-                source = source.iterator(start=next_tid)
+            # Compensate for the RelStorage bug(?) and get a reusable iterator
+            # that starts where we want it to. There's no harm in wrapping it for
+            # other sources like FileStorage too.
+            source = _DefaultStartStorageIteration(source, next_tid)
             log.info("Resuming ZODB copy from %s", readable_tid_repr(next_tid))