Merge pull request #11931 from ClickHouse/aku/local-tmp

Put clickhouse-local data to /tmp by default

programs/local/LocalServer.cpp

@@ -39,12 +39,16 @@
 #include <common/argsToConfig.h>
 #include <Common/TerminalSize.h>
 
+#include <filesystem>
+
 
 namespace DB
 {
 
 namespace ErrorCodes
 {
+    extern const int BAD_ARGUMENTS;
+    extern const int LOGICAL_ERROR;
     extern const int SYNTAX_ERROR;
     extern const int CANNOT_LOAD_CONFIG;
 }
@@ -98,22 +102,55 @@ void LocalServer::applyCmdSettings()
 /// If path is specified and not empty, will try to setup server environment and load existing metadata
 void LocalServer::tryInitPath()
 {
-    std::string path = config().getString("path", "");
-    Poco::trimInPlace(path);
+    std::string path;
 
-    if (!path.empty())
+    if (config().has("path"))
     {
-        if (path.back() != '/')
-            path += '/';
+        // User-supplied path.
+        path = config().getString("path");
+        Poco::trimInPlace(path);
 
-        context->setPath(path);
-        return;
+        if (path.empty())
+        {
+            throw Exception(ErrorCodes::BAD_ARGUMENTS,
+                "Cannot work with emtpy storage path that is explicitly specified"
+                " by the --path option. Please check the program options and"
+                " correct the --path.");
+        }
+    }
+    else
+    {
+        // Default unique path in the system temporary directory.
+        const auto tmp = std::filesystem::temp_directory_path();
+        const auto default_path = tmp
+            / fmt::format("clickhouse-local-{}", getpid());
+
+        if (exists(default_path))
+        {
+            // This is a directory that is left by a previous run of
+            // clickhouse-local that had the same pid and did not complete
+            // correctly. Remove it, with an additional sanity check.
+            if (default_path.parent_path() != tmp)
+            {
+                throw Exception(ErrorCodes::LOGICAL_ERROR,
+                    "The temporary directory of clickhouse-local '{}' is not"
+                    " inside the system temporary directory '{}'. Will not delete"
+                    " it", default_path.string(), tmp.string());
+            }
+
+            remove_all(default_path);
+        }
+
+        create_directory(default_path);
+        temporary_directory_to_delete = default_path;
+
+        path = default_path.string();
     }
 
-    /// In case of empty path set paths to helpful directories
-    std::string cd = Poco::Path::current();
-    context->setTemporaryStorage(cd + "tmp");
-    context->setFlagsPath(cd + "flags");
+    if (path.back() != '/')
+        path += '/';
+
+    context->setPath(path);
     context->setUserFilesPath(""); // user's files are everywhere
 }
 
@@ -228,10 +265,21 @@ try
     context->shutdown();
     context.reset();
 
+    cleanup();
+
     return Application::EXIT_OK;
 }
 catch (const Exception & e)
 {
+    try
+    {
+        cleanup();
+    }
+    catch (...)
+    {
+        tryLogCurrentException(__PRETTY_FUNCTION__);
+    }
+
     std::cerr << getCurrentExceptionMessage(config().hasOption("stacktrace")) << '\n';
 
     /// If exception code isn't zero, we should return non-zero return code anyway.
@@ -372,6 +420,29 @@ void LocalServer::setupUsers()
         throw Exception("Can't load config for users", ErrorCodes::CANNOT_LOAD_CONFIG);
 }
 
+void LocalServer::cleanup()
+{
+    // Delete the temporary directory if needed. Just in case, check that it is
+    // in the system temporary directory, not to delete user data if there is a
+    // bug.
+    if (temporary_directory_to_delete)
+    {
+        const auto tmp = std::filesystem::temp_directory_path();
+        const auto dir = *temporary_directory_to_delete;
+        temporary_directory_to_delete.reset();
+
+        if (dir.parent_path() != tmp)
+        {
+            throw Exception(ErrorCodes::LOGICAL_ERROR,
+                "The temporary directory of clickhouse-local '{}' is not inside"
+                " the system temporary directory '{}'. Will not delete it",
+                dir.string(), tmp.string());
+        }
+
+        remove_all(dir);
+    }
+}
+
 static void showClientVersion()
 {
     std::cout << DBMS_NAME << " client version " << VERSION_STRING << VERSION_OFFICIAL << "." << '\n';

programs/local/LocalServer.h

@@ -2,7 +2,9 @@
 
 #include <Core/Settings.h>
 #include <Poco/Util/Application.h>
+#include <filesystem>
 #include <memory>
+#include <optional>
 #include <loggers/Loggers.h>
 #include <Interpreters/Context.h>
 
@@ -38,6 +40,7 @@ private:
     void applyCmdSettings();
     void processQueries();
     void setupUsers();
+    void cleanup();
 
 protected:
     SharedContextHolder shared_context;
@@ -45,6 +48,8 @@ protected:
 
     /// Settings specified via command line args
     Settings cmd_settings;
+
+    std::optional<std::filesystem::path> temporary_directory_to_delete;
 };
 
 }

src/Interpreters/DatabaseCatalog.cpp

@@ -13,6 +13,8 @@
 #include <IO/ReadHelpers.h>
 #include <Poco/DirectoryIterator.h>
 
+#include <filesystem>
+
 namespace DB
 {
 
@@ -519,6 +521,12 @@ void DatabaseCatalog::loadMarkedAsDroppedTables()
 
     std::map<String, StorageID> dropped_metadata;
     String path = global_context->getPath() + "metadata_dropped/";
+
+    if (!std::filesystem::exists(path))
+    {
+        return;
+    }
+
     Poco::DirectoryIterator dir_end;
     for (Poco::DirectoryIterator it(path); it != dir_end; ++it)
     {

tests/clickhouse-test

@@ -338,14 +338,41 @@ server_logs_level = "warning"
 
 
 def check_server_started(client, retry_count):
+    print("Connecting to ClickHouse server...", end='')
+    sys.stdout.flush()
     while retry_count > 0:
         clickhouse_proc = Popen(shlex.split(client), stdin=PIPE, stdout=PIPE, stderr=PIPE)
         (stdout, stderr) = clickhouse_proc.communicate("SELECT 1")
-        if clickhouse_proc.returncode != 0 or not stdout.startswith("1"):
+
+        if clickhouse_proc.returncode == 0 and stdout.startswith("1"):
+            print(" OK")
+            sys.stdout.flush()
+            return True
+
+        if clickhouse_proc.returncode == 210:
+            # Connection refused, retry
+            print('.', end = '')
+            sys.stdout.flush()
             retry_count -= 1
             sleep(0.5)
-        else:
-            return True
+            continue
+
+        # Other kind of error, fail.
+        print('')
+        print("Client invocation failed with code ", clickhouse_proc.returncode, ": ")
+        # We can't print this, because for some reason this is python 2,
+        # and args appeared in 3.3. To hell with it.
+        # print(''.join(clickhouse_proc.args))
+        print("stdout: ")
+        print(stdout)
+        print("stderr: ")
+        print(stderr)
+        sys.stdout.flush()
+        return False
+
+    print('')
+    print('All connection tries failed')
+    sys.stdout.flush()
 
     return False
 

tests/queries/0_stateless/01146_clickhouse_local_data.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+set -e
+
+CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+. $CUR_DIR/../shell_config.sh
+
+${CLICKHOUSE_LOCAL} --query "create table test engine Log as select 1 a"
+
+# Should not see the table created by the previous instance
+if ${CLICKHOUSE_LOCAL} --query "select * from test" 2>/dev/null
+then
+ exit 1
+fi
+