You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by jd...@apache.org on 2017/10/30 16:20:40 UTC

[1/2] kudu git commit: fs: ensure the existence of important directories on open

Repository: kudu
Updated Branches:
  refs/heads/master 13908bd9f -> c08f27fc5


fs: ensure the existence of important directories on open

Seems like a pretty easy way to improve robustness, especially for CLI tools
that may not access these directories directly but nevertheless want to
guarantee their existence before making destructive changes.

Change-Id: I5238987273886421f8e6f87f850183dc81c8c4ec
Reviewed-on: http://gerrit.cloudera.org:8080/8399
Tested-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/9a4ea1c7
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/9a4ea1c7
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/9a4ea1c7

Branch: refs/heads/master
Commit: 9a4ea1c7fa1cc6c7345f012f35e7681364ed3534
Parents: 13908bd
Author: Adar Dembo <ad...@cloudera.com>
Authored: Thu Oct 26 10:32:05 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Thu Oct 26 23:04:50 2017 +0000

----------------------------------------------------------------------
 src/kudu/fs/fs_manager-test.cc   | 16 ++++++++++++++++
 src/kudu/fs/fs_manager.cc        | 14 ++++++++++++++
 src/kudu/tools/kudu-tool-test.cc | 14 ++++++++------
 3 files changed, 38 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/9a4ea1c7/src/kudu/fs/fs_manager-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager-test.cc b/src/kudu/fs/fs_manager-test.cc
index 6ed4f93..2d011d9 100644
--- a/src/kudu/fs/fs_manager-test.cc
+++ b/src/kudu/fs/fs_manager-test.cc
@@ -400,4 +400,20 @@ TEST_F(FsManagerTestBase, TestUmask) {
   EXPECT_EQ("700", FilePermsAsString(root));
 }
 
+TEST_F(FsManagerTestBase, TestOpenFailsWhenMissingImportantDir) {
+  const string kWalRoot = fs_manager()->GetWalsRootDir();
+
+  ASSERT_OK(env_->DeleteDir(kWalRoot));
+  ReinitFsManager();
+  Status s = fs_manager()->Open();
+  ASSERT_TRUE(s.IsNotFound());
+  ASSERT_STR_CONTAINS(s.ToString(), "could not verify required directory");
+
+  unique_ptr<WritableFile> f;
+  ASSERT_OK(env_->NewWritableFile(kWalRoot, &f));
+  s = fs_manager()->Open();
+  ASSERT_TRUE(s.IsCorruption());
+  ASSERT_STR_CONTAINS(s.ToString(), "exists but is not a directory");
+}
+
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/9a4ea1c7/src/kudu/fs/fs_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index 5f62858..e9df4e3 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -307,6 +307,20 @@ Status FsManager::Open(FsReport* report) {
     }
   }
 
+  // Ensure all of the ancillary directories exist.
+  vector<string> ancillary_dirs = { GetWalsRootDir(),
+                                    GetTabletMetadataDir(),
+                                    GetConsensusMetadataDir() };
+  for (const auto& d : ancillary_dirs) {
+    bool is_dir;
+    RETURN_NOT_OK_PREPEND(env_->IsDirectory(d, &is_dir),
+                          Substitute("could not verify required directory $0", d));
+    if (!is_dir) {
+      return Status::Corruption(
+          Substitute("Required directory $0 exists but is not a directory", d));
+    }
+  }
+
   // Remove leftover temporary files from the WAL root and fix permissions.
   //
   // Temporary files in the data directory roots will be removed by the block

http://git-wip-us.apache.org/repos/asf/kudu/blob/9a4ea1c7/src/kudu/tools/kudu-tool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 0df6dc2..0edd1e3 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -67,6 +67,7 @@
 #include "kudu/gutil/port.h"
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/gutil/stl_util.h"
+#include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/numbers.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/strcat.h"
@@ -639,8 +640,9 @@ TEST_F(ToolTest, TestFsCheck) {
 
 TEST_F(ToolTest, TestFsCheckLiveServer) {
   NO_FATALS(StartExternalMiniCluster());
-  string master_data_dir = cluster_->GetDataPath("master-0");
-  string args = Substitute("fs check --fs_wal_dir $0", master_data_dir);
+  string args = Substitute("fs check --fs_wal_dir $0 --fs_data_dirs $1",
+                           cluster_->GetWalPath("master-0"),
+                           JoinStrings(cluster_->GetDataPaths("master-0"), ","));
   NO_FATALS(RunFsCheck(args, 0, "", {}, 0));
   args += " --repair";
   string stdout;
@@ -1421,11 +1423,11 @@ TEST_F(ToolTest, TestRemoteReplicaCopy) {
   // Shut down the destination server and delete one tablet from
   // local fs on destination tserver while it is offline.
   cluster_->tablet_server(kDstTsIndex)->Shutdown();
-  const string& tserver_dir = cluster_->tablet_server(kDstTsIndex)->data_dir();
   const string& deleted_tablet_id = tablets[1].tablet_status().tablet_id();
-  NO_FATALS(RunActionStdoutNone(Substitute("local_replica delete $0 --fs_wal_dir=$1 "
-                                           "--fs_data_dirs=$1 --clean_unsafe",
-                                           deleted_tablet_id, tserver_dir)));
+  NO_FATALS(RunActionStdoutNone(Substitute(
+      "local_replica delete $0 --fs_wal_dir=$1 --fs_data_dirs=$2 --clean_unsafe",
+      deleted_tablet_id, cluster_->tablet_server(kDstTsIndex)->wal_dir(),
+      JoinStrings(cluster_->tablet_server(kDstTsIndex)->data_dirs(), ","))));
 
   // At this point, we expect only 2 tablets to show up on destination when
   // we restart the destination tserver. deleted_tablet_id should not be found on


[2/2] kudu git commit: python: upgrade pip when building client and restore old workarounds

Posted by jd...@apache.org.
python: upgrade pip when building client and restore old workarounds

It appears that pypi recently deactivated their http:// endpoint, which is
the default in the older versions of pip found on el6. As a result, pip
installation requests fail:

  Downloading/unpacking pytest (from -r requirements.txt (line 1))
    Cannot fetch index base URL http://pypi.python.org/simple/
    Could not find any downloads that satisfy the requirement pytest (from -r requirements.txt (line 1))
  No distributions at all found for pytest (from -r requirements.txt (line 1))

Passing -i with the https:// endpoint works for the dependencies being
installed, but it doesn't appear to be passed down correctly to transitive
dependencies, and they fail to install with an inscrutable error [1].

So instead, let's go back to the old approach where we first upgrade pip.
This means effectively reverting [2] and restoring [3], but such is life.

I tested this by running build-and-test.sh on an el6 machine that created
virtualenvs containing pip 1.1 and setuptools 0.6c11.

1. https://stackoverflow.com/questions/5178535/easy-install-libmysqld-dev-error-nonetype-object-has-no-attribute-clone
2. https://github.com/apache/kudu/commit/b198ed8ff6a603816892c8bc7fd80679a014aaf1
3. https://github.com/apache/kudu/commit/d0acb55510c0552605953e07740f46d6be66c9c1

Change-Id: Ia75b65d66a1559ddaefb53dc1e67837faf9e7e6a
Reviewed-on: http://gerrit.cloudera.org:8080/8406
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/c08f27fc
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/c08f27fc
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/c08f27fc

Branch: refs/heads/master
Commit: c08f27fc583cbce2c99e4b28c9ddb91e9c9e1e69
Parents: 9a4ea1c
Author: Adar Dembo <ad...@cloudera.com>
Authored: Fri Oct 27 14:23:19 2017 -0700
Committer: Jean-Daniel Cryans <jd...@apache.org>
Committed: Mon Oct 30 16:02:20 2017 +0000

----------------------------------------------------------------------
 build-support/jenkins/build-and-test.sh | 44 ++++++++++++++++++++++++++--
 python/requirements.txt                 |  1 -
 2 files changed, 42 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/c08f27fc/build-support/jenkins/build-and-test.sh
----------------------------------------------------------------------
diff --git a/build-support/jenkins/build-and-test.sh b/build-support/jenkins/build-and-test.sh
index b303051..c2b0717 100755
--- a/build-support/jenkins/build-and-test.sh
+++ b/build-support/jenkins/build-and-test.sh
@@ -416,7 +416,27 @@ if [ "$BUILD_PYTHON" == "1" ]; then
   virtualenv $KUDU_BUILD/py_env
   source $KUDU_BUILD/py_env/bin/activate
 
-  # Download all necessary requirements.
+  # Old versions of pip (such as the one found in el6) default to pypi's http://
+  # endpoint which no longer exists. The -i option lets us switch to the
+  # https:// endpoint in such cases.
+  #
+  # Unfortunately, in these old versions of pip, -i doesn't appear to apply
+  # recursively to transitive dependencies installed via a direct dependency's
+  # "python setup.py" command. Therefore we have no choice but to upgrade to a
+  # new version of pip to proceed.
+  pip install -i https://pypi.python.org/simple --upgrade pip
+
+  # New versions of pip raise an exception when upgrading old versions of
+  # setuptools (such as the one found in el6). The workaround is to upgrade
+  # setuptools on its own, outside of requirements.txt, and with the pip version
+  # check disabled.
+  pip install --disable-pip-version-check --upgrade 'setuptools >= 0.8'
+
+  # We've got a new pip and new setuptools. We can now install the rest of the
+  # Python client's requirements.
+  #
+  # Installing the Cython dependency may involve some compiler work, so we must
+  # pass in the current values of CC and CXX.
   CC=$CLANG CXX=$CLANG++ pip install -r requirements.txt
 
   # Delete old Cython extensions to force them to be rebuilt.
@@ -454,7 +474,27 @@ if [ "$BUILD_PYTHON3" == "1" ]; then
   virtualenv -p python3 $KUDU_BUILD/py_env
   source $KUDU_BUILD/py_env/bin/activate
 
-  # Download all necessary requirements.
+  # Old versions of pip (such as the one found in el6) default to pypi's http://
+  # endpoint which no longer exists. The -i option lets us switch to the
+  # https:// endpoint in such cases.
+  #
+  # Unfortunately, in these old versions of pip, -i doesn't appear to apply
+  # recursively to transitive dependencies installed via a direct dependency's
+  # "python setup.py" command. Therefore we have no choice but to upgrade to a
+  # new version of pip to proceed.
+  pip install -i https://pypi.python.org/simple --upgrade pip
+
+  # New versions of pip raise an exception when upgrading old versions of
+  # setuptools (such as the one found in el6). The workaround is to upgrade
+  # setuptools on its own, outside of requirements.txt, and with the pip version
+  # check disabled.
+  pip install --disable-pip-version-check --upgrade 'setuptools >= 0.8'
+
+  # We've got a new pip and new setuptools. We can now install the rest of the
+  # Python client's requirements.
+  #
+  # Installing the Cython dependency may involve some compiler work, so we must
+  # pass in the current values of CC and CXX.
   CC=$CLANG CXX=$CLANG++ pip install -r requirements.txt
 
   # Delete old Cython extensions to force them to be rebuilt.

http://git-wip-us.apache.org/repos/asf/kudu/blob/c08f27fc/python/requirements.txt
----------------------------------------------------------------------
diff --git a/python/requirements.txt b/python/requirements.txt
index ecd5284..1740925 100644
--- a/python/requirements.txt
+++ b/python/requirements.txt
@@ -1,6 +1,5 @@
 pytest
 cython == 0.26.1
-setuptools >= 0.8
 six
 unittest2
 pytz