Retrying FileExists errors in case of errors to overcome intermittent GCS errors.

tensorflower-gardener · Jonathan Hseu · commit aee4914fadca · 2017-03-03T13:23:22.000-08:00
Change: 149038920
diff --git a/tensorflow/core/platform/cloud/retrying_file_system.cc b/tensorflow/core/platform/cloud/retrying_file_system.cc
@@ -131,8 +131,10 @@ Status RetryingFileSystem::NewReadOnlyMemoryRegionFromFile(
 }
 
 Status RetryingFileSystem::FileExists(const string& fname) {
-  // No status -- no retries.
-  return base_file_system_->FileExists(fname);
+  return RetryingUtils::CallWithRetries(
+      std::bind(&FileSystem::FileExists, base_file_system_.get(), fname),
+      initial_delay_microseconds_);
+  ;
 }
 
 Status RetryingFileSystem::Stat(const string& fname, FileStatistics* stat) {
diff --git a/tensorflow/core/platform/cloud/retrying_file_system_test.cc b/tensorflow/core/platform/cloud/retrying_file_system_test.cc
@@ -109,7 +109,9 @@ class MockFileSystem : public FileSystem {
     return calls_.ConsumeNextCall("NewReadOnlyMemoryRegionFromFile");
   }
 
-  Status FileExists(const string& fname) override { return Status::OK(); }
+  Status FileExists(const string& fname) override {
+    return calls_.ConsumeNextCall("FileExists");
+  }
 
   Status GetChildren(const string& dir, std::vector<string>* result) override {
     return calls_.ConsumeNextCall("GetChildren");
@@ -619,6 +621,26 @@ TEST(RetryingFileSystemTest, FileExists_SuccessWith2ndTry) {
   TF_EXPECT_OK(fs.FileExists("gs://path/dir"));
 }
 
+TEST(RetryingFileSystemTest, FileExists_AllRetriesFailed) {
+  ExpectedCalls expected_fs_calls = CreateRetriableErrors("FileExists", 6);
+  std::unique_ptr<MockFileSystem> base_fs(
+      new MockFileSystem(expected_fs_calls));
+  RetryingFileSystem fs(std::move(base_fs), 0);
+
+  EXPECT_EQ("Retriable error #5", fs.FileExists("file_name").error_message());
+}
+
+TEST(RetryingFileSystemTest, FileExists_SuccessWith2ndTry) {
+  ExpectedCalls expected_fs_calls(
+      {std::make_tuple("FileExists", errors::Unavailable("Something is wrong")),
+       std::make_tuple("FileExists", Status::OK())});
+  std::unique_ptr<MockFileSystem> base_fs(
+      new MockFileSystem(expected_fs_calls));
+  RetryingFileSystem fs(std::move(base_fs), 0);
+
+  TF_EXPECT_OK(fs.FileExists("gs://path/dir"));
+}
+
 TEST(RetryingFileSystemTest, IsDirectory_SuccessWith2ndTry) {
   ExpectedCalls expected_fs_calls(
       {std::make_tuple("IsDirectory",

Original file line number	Diff line number	Diff line change
`@@ -131,8 +131,10 @@ Status RetryingFileSystem::NewReadOnlyMemoryRegionFromFile(`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`Status RetryingFileSystem::FileExists(const string& fname) {`
`134`		`- // No status -- no retries.`
`135`		`- return base_file_system_->FileExists(fname);`
	`134`	`+ return RetryingUtils::CallWithRetries(`
	`135`	`+ std::bind(&FileSystem::FileExists, base_file_system_.get(), fname),`
	`136`	`+ initial_delay_microseconds_);`
	`137`	`+ ;`
`136`	`138`	`}`
`137`	`139`
`138`	`140`	`Status RetryingFileSystem::Stat(const string& fname, FileStatistics* stat) {`