[ARVADOS] updated: d2a202effce8975313f5fc92fd5dfbb72bd56034

Git user git at public.curoverse.com
Thu Feb 23 15:59:57 EST 2017


Summary of changes:
 sdk/python/arvados/arvfile.py      | 12 +++++++++++-
 sdk/python/arvados/commands/put.py |  8 ++++++--
 sdk/python/tests/test_arv_put.py   | 28 +++++++++++++++++++++++-----
 3 files changed, 40 insertions(+), 8 deletions(-)

       via  d2a202effce8975313f5fc92fd5dfbb72bd56034 (commit)
       via  56a99279c3fa45c95db7daa726a3fc0c071dffe1 (commit)
      from  a83e4de23b64a24e89321714511f6d9ef4ce4831 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit d2a202effce8975313f5fc92fd5dfbb72bd56034
Author: Lucas Di Pentima <lucas at curoverse.com>
Date:   Thu Feb 23 17:56:24 2017 -0300

    11002: Don't save the state and log the stack trace before quitting upon catching an exception. Also, when receiving SIGINT (KeyboardInterrupt), just quit without any logging.
    Updated tests to reflect this new behaviour.

diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py
index 5598e54..a6c7d14 100644
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -23,6 +23,8 @@ import sys
 import tempfile
 import threading
 import time
+import traceback
+
 from apiclient import errors as apiclient_errors
 from arvados._version import __version__
 
@@ -448,9 +450,11 @@ class ArvPutUploadJob(object):
             # Actual file upload
             self._upload_started = True # Used by the update thread to start checkpointing
             self._upload_files()
-        except KeyboardInterrupt:
-            self.logger.warning("User interrupt request, cleaning up before exiting.")
+        except (SystemExit, Exception) as e:
             self._checkpoint_before_quit = False
+            # Log stack trace only when Ctrl-C isn't pressed (SIGINT)
+            if not isinstance(e, SystemExit) or e.code != -2:
+                self.logger.warning("Abnormal termination:\n{}".format(traceback.format_exc(e)))
             raise
         finally:
             if not self.dry_run:
diff --git a/sdk/python/tests/test_arv_put.py b/sdk/python/tests/test_arv_put.py
index 108d661..286a22e 100644
--- a/sdk/python/tests/test_arv_put.py
+++ b/sdk/python/tests/test_arv_put.py
@@ -322,6 +322,8 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             data = args[1]
             # Exit only on last block
             if len(data) < arvados.config.KEEP_BLOCK_SIZE:
+                # Simulate a checkpoint before quitting. Ensure block commit.
+                self.writer._update(final=True)
                 raise SystemExit("Simulated error")
             return self.arvfile_write(*args, **kwargs)
 
@@ -330,6 +332,8 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             mocked_write.side_effect = wrapped_write
             writer = arv_put.ArvPutUploadJob([self.large_file_name],
                                              replication_desired=1)
+            # We'll be accessing from inside the wrapper
+            self.writer = writer
             with self.assertRaises(SystemExit):
                 writer.start(save_collection=False)
             # Confirm that the file was partially uploaded
@@ -343,6 +347,7 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
         self.assertEqual(writer.bytes_written + writer2.bytes_written - writer2.bytes_skipped,
                          os.path.getsize(self.large_file_name))
         writer2.destroy_cache()
+        del(self.writer)
 
     # Test for bug #11002
     def test_graceful_exit_while_repacking_small_blocks(self):
@@ -370,6 +375,8 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             data = args[1]
             # Exit only on last block
             if len(data) < arvados.config.KEEP_BLOCK_SIZE:
+                # Simulate a checkpoint before quitting.
+                self.writer._update()
                 raise SystemExit("Simulated error")
             return self.arvfile_write(*args, **kwargs)
 
@@ -378,6 +385,8 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             mocked_write.side_effect = wrapped_write
             writer = arv_put.ArvPutUploadJob([self.large_file_name],
                                              replication_desired=1)
+            # We'll be accessing from inside the wrapper
+            self.writer = writer
             with self.assertRaises(SystemExit):
                 writer.start(save_collection=False)
             # Confirm that the file was partially uploaded
@@ -393,12 +402,15 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
         self.assertEqual(writer2.bytes_written,
                          os.path.getsize(self.large_file_name))
         writer2.destroy_cache()
+        del(self.writer)
 
     def test_no_resume_when_no_cache(self):
         def wrapped_write(*args, **kwargs):
             data = args[1]
             # Exit only on last block
             if len(data) < arvados.config.KEEP_BLOCK_SIZE:
+                # Simulate a checkpoint before quitting.
+                self.writer._update()
                 raise SystemExit("Simulated error")
             return self.arvfile_write(*args, **kwargs)
 
@@ -407,6 +419,8 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             mocked_write.side_effect = wrapped_write
             writer = arv_put.ArvPutUploadJob([self.large_file_name],
                                              replication_desired=1)
+            # We'll be accessing from inside the wrapper
+            self.writer = writer
             with self.assertRaises(SystemExit):
                 writer.start(save_collection=False)
             # Confirm that the file was partially uploaded
@@ -423,13 +437,15 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
         self.assertEqual(writer2.bytes_written,
                          os.path.getsize(self.large_file_name))
         writer2.destroy_cache()
-
+        del(self.writer)
 
     def test_dry_run_feature(self):
         def wrapped_write(*args, **kwargs):
             data = args[1]
             # Exit only on last block
             if len(data) < arvados.config.KEEP_BLOCK_SIZE:
+                # Simulate a checkpoint before quitting.
+                self.writer._update()
                 raise SystemExit("Simulated error")
             return self.arvfile_write(*args, **kwargs)
 
@@ -438,6 +454,8 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             mocked_write.side_effect = wrapped_write
             writer = arv_put.ArvPutUploadJob([self.large_file_name],
                                              replication_desired=1)
+            # We'll be accessing from inside the wrapper
+            self.writer = writer
             with self.assertRaises(SystemExit):
                 writer.start(save_collection=False)
             # Confirm that the file was partially uploaded
@@ -473,7 +491,7 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
                                     replication_desired=1,
                                     dry_run=True,
                                     resume=False)
-
+        del(self.writer)
 
 class ArvadosExpectedBytesTest(ArvadosBaseTestCase):
     TEST_SIZE = os.path.getsize(__file__)

commit 56a99279c3fa45c95db7daa726a3fc0c071dffe1
Author: Lucas Di Pentima <lucas at curoverse.com>
Date:   Wed Feb 22 15:26:16 2017 -0300

    11002: Track this specific error with its own exception class, for future-proofing.

diff --git a/sdk/python/arvados/arvfile.py b/sdk/python/arvados/arvfile.py
index edeb910..dcfb427 100644
--- a/sdk/python/arvados/arvfile.py
+++ b/sdk/python/arvados/arvfile.py
@@ -38,6 +38,12 @@ def split(path):
         stream_name, file_name = '.', path
     return stream_name, file_name
 
+
+class UnownedBlockError(Exception):
+    """Raised when there's an writable block without an owner on the BlockManager."""
+    pass
+
+
 class _FileLikeObjectBase(object):
     def __init__(self, name, mode):
         self.name = name
@@ -571,7 +577,11 @@ class _BlockManager(object):
             # A WRITABLE block always has an owner.
             # A WRITABLE block with its owner.closed() implies that it's
             # size is <= KEEP_BLOCK_SIZE/2.
-            small_blocks = [b for b in self._bufferblocks.values() if b.state() == _BufferBlock.WRITABLE and b.owner.closed()]
+            try:
+                small_blocks = [b for b in self._bufferblocks.values() if b.state() == _BufferBlock.WRITABLE and b.owner.closed()]
+            except AttributeError:
+                # Writable blocks without owner shouldn't exist.
+                raise UnownedBlockError()
 
             if len(small_blocks) <= 1:
                 # Not enough small blocks for repacking
diff --git a/sdk/python/tests/test_arv_put.py b/sdk/python/tests/test_arv_put.py
index f0c76c2..108d661 100644
--- a/sdk/python/tests/test_arv_put.py
+++ b/sdk/python/tests/test_arv_put.py
@@ -347,7 +347,7 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
     # Test for bug #11002
     def test_graceful_exit_while_repacking_small_blocks(self):
         def wrapped_commit(*args, **kwargs):
-            raise KeyboardInterrupt("Simulated error")
+            raise SystemExit("Simulated error")
 
         with mock.patch('arvados.arvfile._BlockManager.commit_bufferblock',
                         autospec=True) as mocked_commit:
@@ -359,9 +359,9 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             writer = arv_put.ArvPutUploadJob([self.small_files_dir],
                                              replication_desired=1)
             try:
-                with self.assertRaises(KeyboardInterrupt):
+                with self.assertRaises(SystemExit):
                     writer.start(save_collection=False)
-            except AttributeError:
+            except arvados.arvfile.UnownedBlockError:
                 self.fail("arv-put command is trying to use a corrupted BlockManager. See https://dev.arvados.org/issues/11002")
         writer.destroy_cache()
 

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list