Normalize "pid" file handling #3501

base/daemon/BaseDaemon.cpp

@@ -628,7 +628,7 @@ void BaseDaemon::initialize(Application & self)
 
     /// Create pid file.
     if (config().has("pid"))
-        pid.emplace(config().getString("pid"));
+        pid.emplace(config().getString("pid"), DB::StatusFile::write_pid);
 
     /// Change path for logging.
     if (!log_path.empty())
@@ -812,63 +812,6 @@ void BaseDaemon::defineOptions(Poco::Util::OptionSet & new_options)
     Poco::Util::ServerApplication::defineOptions(new_options);
 }
 
-bool isPidRunning(pid_t pid)
-{
-    return getpgid(pid) >= 0;
-}
-
-BaseDaemon::PID::PID(const std::string & file_)
-{
-    file = Poco::Path(file_).absolute().toString();
-    Poco::File poco_file(file);
-
-    if (poco_file.exists())
-    {
-        pid_t pid_read = 0;
-        {
-            std::ifstream in(file);
-            if (in.good())
-            {
-                in >> pid_read;
-                if (pid_read && isPidRunning(pid_read))
-                    throw Poco::Exception("Pid file exists and program running with pid = " + std::to_string(pid_read) + ", should not start daemon.");
-            }
-        }
-        std::cerr << "Old pid file exists (with pid = " << pid_read << "), removing." << std::endl;
-        poco_file.remove();
-    }
-
-    int fd = open(file.c_str(),
-        O_CREAT | O_EXCL | O_WRONLY,
-        S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
-
-    if (-1 == fd)
-    {
-        if (EEXIST == errno)
-            throw Poco::Exception("Pid file exists, should not start daemon.");
-        throw Poco::CreateFileException("Cannot create pid file.");
-    }
-
-    SCOPE_EXIT({ close(fd); });
-
-    std::stringstream s;
-    s << getpid();
-    if (static_cast<ssize_t>(s.str().size()) != write(fd, s.str().c_str(), s.str().size()))
-        throw Poco::Exception("Cannot write to pid file.");
-}
-
-BaseDaemon::PID::~PID()
-{
-    try
-    {
-        Poco::File(file).remove();
-    }
-    catch (...)
-    {
-        DB::tryLogCurrentException(__PRETTY_FUNCTION__);
-    }
-}
-
 void BaseDaemon::handleSignal(int signal_id)
 {
     if (signal_id == SIGINT ||

base/daemon/BaseDaemon.h

@@ -22,6 +22,7 @@
 #include <common/getThreadId.h>
 #include <daemon/GraphiteWriter.h>
 #include <Common/Config/ConfigProcessor.h>
+#include <Common/StatusFile.h>
 #include <loggers/Loggers.h>
 
 
@@ -163,16 +164,7 @@ protected:
 
     std::unique_ptr<Poco::TaskManager> task_manager;
 
-    /// RAII wrapper for pid file.
+    std::optional<DB::StatusFile> pid;
-    struct PID
-    {
-        std::string file;
-
-        PID(const std::string & file_);
-        ~PID();
-    };
-
-    std::optional<PID> pid;
 
     std::atomic_bool is_cancelled{false};
 

programs/copier/ClusterCopierApp.cpp

@@ -1,4 +1,6 @@
 #include "ClusterCopierApp.h"
+#include <Common/StatusFile.h>
+
 
 namespace DB
 {
@@ -91,7 +93,7 @@ void ClusterCopierApp::defineOptions(Poco::Util::OptionSet & options)
 
 void ClusterCopierApp::mainImpl()
 {
-    StatusFile status_file(process_path + "/status");
+    StatusFile status_file(process_path + "/status", StatusFile::write_full_info);
     ThreadStatus thread_status;
 
     auto * log = &logger();

programs/copier/Internals.h

@@ -66,7 +66,6 @@
 #include <Dictionaries/registerDictionaries.h>
 #include <Disks/registerDisks.h>
 #include <Databases/DatabaseMemory.h>
-#include <Common/StatusFile.h>
 
 #include "Aliases.h"
 

programs/local/LocalServer.cpp

@@ -248,7 +248,7 @@ try
     if (!context->getPath().empty())
     {
         /// Lock path directory before read
-        status.emplace(context->getPath() + "status");
+        status.emplace(context->getPath() + "status", StatusFile::write_full_info);
 
         LOG_DEBUG(log, "Loading metadata from {}", context->getPath());
         loadMetadataSystem(*context);

programs/server/Server.cpp

@@ -378,7 +378,7 @@ int Server::main(const std::vector<std::string> & /*args*/)
 
     global_context->setPath(path);
 
-    StatusFile status{path + "status"};
+    StatusFile status{path + "status", StatusFile::write_full_info};
 
     SCOPE_EXIT({
         /** Ask to cancel background jobs all table engines,

src/Common/StatusFile.cpp

@@ -30,8 +30,21 @@ namespace ErrorCodes
 }
 
 
-StatusFile::StatusFile(const std::string & path_)
+StatusFile::FillFunction StatusFile::write_pid = [](WriteBuffer & out)
-    : path(path_)
+{
+    out << getpid() << "\n";
+};
+
+StatusFile::FillFunction StatusFile::write_full_info = [](WriteBuffer & out)
+{
+    out << "PID: " << getpid() << "\n"
+        << "Started at: " << LocalDateTime(time(nullptr)) << "\n"
+        << "Revision: " << ClickHouseRevision::get() << "\n";
+};
+
+
+StatusFile::StatusFile(std::string path_, FillFunction fill_)
+    : path(std::move(path_)), fill(std::move(fill_))
 {
     /// If file already exists. NOTE Minor race condition.
     if (Poco::File(path).exists())
@@ -72,13 +85,8 @@ StatusFile::StatusFile(const std::string & path_)
             throwFromErrnoWithPath("Cannot lseek " + path, path, ErrorCodes::CANNOT_SEEK_THROUGH_FILE);
 
         /// Write information about current server instance to the file.
-        {
+        WriteBufferFromFileDescriptor out(fd, 1024);
-            WriteBufferFromFileDescriptor out(fd, 1024);
+        fill(out);
-            out
-                << "PID: " << getpid() << "\n"
-                << "Started at: " << LocalDateTime(time(nullptr)) << "\n"
-                << "Revision: " << ClickHouseRevision::get() << "\n";
-        }
     }
     catch (...)
     {

src/Common/StatusFile.h

@@ -1,23 +1,33 @@
 #pragma once
 
 #include <string>
+#include <functional>
 #include <boost/noncopyable.hpp>
 
 
 namespace DB
 {
 
+class WriteBuffer;
+
 
 /** Provides that no more than one server works with one data directory.
   */
 class StatusFile : private boost::noncopyable
 {
 public:
-    explicit StatusFile(const std::string & path_);
+    using FillFunction = std::function<void(WriteBuffer&)>;
+
+    StatusFile(std::string path_, FillFunction fill_);
     ~StatusFile();
 
+    /// You can use one of these functions to fill the file or provide your own.
+    static FillFunction write_pid;
+    static FillFunction write_full_info;
+
 private:
     const std::string path;
+    FillFunction fill;
     int fd = -1;
 };