You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bb...@apache.org on 2022/02/22 19:51:19 UTC

[geode-native] branch develop updated: GEODE-9325: Replace old ITs worker spawner (#813)

This is an automated email from the ASF dual-hosted git repository.

bbender pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git


The following commit(s) were added to refs/heads/develop by this push:
     new 73a7c52  GEODE-9325: Replace old ITs worker spawner (#813)
73a7c52 is described below

commit 73a7c524e4fdebacd8513c0bb4180ea5ede2c8ad
Author: Mario Salazar de Torres <ma...@est.tech>
AuthorDate: Tue Feb 22 20:51:13 2022 +0100

    GEODE-9325: Replace old ITs worker spawner (#813)
    
    - Replaced references to ACE_Process realated to old ITs process
       spawners.
     - Added Boost program_options project, given that was needed to fulfill
       above changes.
     - Fix Windows execution whenever a test failed. Apparently if not all
     the processes have terminated ctest does not exit its execution and due
     to the fact that there is a worker in charge of managing the cluster
     processes, if the coordinator terminates all of the workers, it's not
     guaranteed that the cluster is stopped.
     - Reversed order in which cleanups are called in order to ensure proper
     teardown.
---
 cppcache/integration-test/ClientCleanup.cpp |  2 +-
 cppcache/integration-test/ClientCleanup.hpp |  4 +-
 cppcache/integration-test/fw_dunit.cpp      | 60 +++++++++++++------
 cppcache/integration-test/fw_spawn.hpp      | 93 -----------------------------
 4 files changed, 44 insertions(+), 115 deletions(-)

diff --git a/cppcache/integration-test/ClientCleanup.cpp b/cppcache/integration-test/ClientCleanup.cpp
index 2727174..e6a2577 100644
--- a/cppcache/integration-test/ClientCleanup.cpp
+++ b/cppcache/integration-test/ClientCleanup.cpp
@@ -32,5 +32,5 @@ void ClientCleanup::trigger() {
 }
 
 void ClientCleanup::registerCallback(std::function<void()> callback) {
-  callbacks_.emplace_back(callback);
+  callbacks_.emplace_front(callback);
 }
diff --git a/cppcache/integration-test/ClientCleanup.hpp b/cppcache/integration-test/ClientCleanup.hpp
index 33cb949..f9c3536 100644
--- a/cppcache/integration-test/ClientCleanup.hpp
+++ b/cppcache/integration-test/ClientCleanup.hpp
@@ -21,7 +21,7 @@
 #define GEODE_INTEGRATION_TEST_CLIENT_CLEANUP_H_
 
 #include <functional>
-#include <vector>
+#include <list>
 
 class ClientCleanup {
  public:
@@ -29,7 +29,7 @@ class ClientCleanup {
   void registerCallback(std::function<void()> callback);
 
  protected:
-  std::vector<std::function<void()>> callbacks_;
+  std::list<std::function<void()>> callbacks_;
 };
 
 #endif  // GEODE_INTEGRATION_TEST_CLIENT_CLEANUP_H_
diff --git a/cppcache/integration-test/fw_dunit.cpp b/cppcache/integration-test/fw_dunit.cpp
index a6d927b..177f3ea 100644
--- a/cppcache/integration-test/fw_dunit.cpp
+++ b/cppcache/integration-test/fw_dunit.cpp
@@ -16,11 +16,6 @@
  * limitations under the License.
  */
 
-#ifdef _WIN32
-// TODO. Remove this whenever ACE_Process is removed
-#define FD_SETSIZE 1024
-#endif
-
 #ifdef USE_SMARTHEAP
 #include <smrtheap.h>
 #endif
@@ -41,7 +36,6 @@
 #include <boost/interprocess/shared_memory_object.hpp>
 #endif
 
-#include "fw_spawn.hpp"
 #include "fwklib/FwkException.hpp"
 
 #define __DUNIT_NO_MAIN__
@@ -361,19 +355,48 @@ void Task::setTimeout(int seconds) {
   }
 }
 
-class TestProcess : virtual public dunit::Manager {
- private:
-  WorkerId m_sId;
-
+class TestProcess {
  public:
   TestProcess(const std::string &cmdline, uint32_t id)
-      : Manager(cmdline), m_sId(id) {}
+      : id_{id}, running_{false}, cmd_{cmdline} {}
 
-  WorkerId &getWorkerId() { return m_sId; }
+  WorkerId &getWorkerId() { return id_; }
+
+  void run() {
+    auto arguments = bpo::split_unix(cmd_);
+
+    std::string exe = arguments[0];
+    arguments.erase(arguments.begin());
+    process_ = bp::child(exe, bp::args = arguments);
+
+    process_.wait();
+    if (process_.exit_code() != 0) {
+      std::clog << "Worker " << id_.getIdName() << " exited with code "
+                << process_.exit_code() << std::endl;
+    }
+
+    running_ = false;
+  }
+
+  void start() {
+    running_ = true;
+    thread_ = std::thread{[this]() { run(); }};
+  }
+
+  void stop() {
+    if (thread_.joinable()) {
+      thread_.join();
+    }
+  }
+
+  bool running() const { return running_; }
 
  protected:
- public:
-  ~TestProcess() noexcept override = default;
+  WorkerId id_;
+  bool running_;
+  std::string cmd_;
+  bp::child process_;
+  std::thread thread_;
 };
 
 /**
@@ -398,7 +421,7 @@ class TestDriver {
     std::cout << std::flush;
     // start each of the workers...
     for (uint32_t j = 1; j < 5; j++) {
-      m_workers[j - 1]->doWork();
+      m_workers[j - 1]->start();
       std::this_thread::sleep_for(
           std::chrono::seconds(2));  // do not increase this to avoid precheckin
                                      // runs taking much longer.
@@ -406,13 +429,12 @@ class TestDriver {
   }
 
   ~TestDriver() {
-    // kill off any children that have not yet terminated.
     for (uint32_t i = 0; i < TestState::WORKER_COUNT;) {
       auto worker = m_workers[i++];
-      if (worker->running() == 1) {
-        delete worker;  // worker destructor should terminate process.
-      }
+      worker->stop();
+      delete worker;
     }
+
     dunit::Dunit::close();
   }
 
diff --git a/cppcache/integration-test/fw_spawn.hpp b/cppcache/integration-test/fw_spawn.hpp
deleted file mode 100644
index 8ebee0e..0000000
--- a/cppcache/integration-test/fw_spawn.hpp
+++ /dev/null
@@ -1,93 +0,0 @@
-#pragma once
-
-#ifndef GEODE_INTEGRATION_TEST_FW_SPAWN_H_
-#define GEODE_INTEGRATION_TEST_FW_SPAWN_H_
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-// Spawn.cpp,v 1.4 2004/01/07 22:40:16 shuston Exp
-
-// @TODO, this out this include list..
-
-#include <ace/Process.h>
-#include <ace/Log_Msg.h>
-#include <boost/iostreams/device/file_descriptor.hpp>
-
-  namespace dunit {
-
-  // Listing 1 code/ch10
-  class Manager : virtual public ACE_Process {
-   public:
-    explicit Manager(const std::string &program_name)
-        : ACE_Process{}, programName_{program_name} {}
-
-    virtual int doWork(void) {
-      // Spawn the new process; prepare() hook is called first.
-      ACE_Process_Options options;
-      pid_t pid = this->spawn(options);
-      if (pid == -1) {
-        ACE_ERROR_RETURN((LM_ERROR, ACE_TEXT("%p\n"), ACE_TEXT("spawn")), -1);
-      }
-      return pid;
-    }
-
-   protected:
-    int prepare(ACE_Process_Options &options) override {
-      options.command_line("%s", this->programName_.c_str());
-      if (this->setStdHandles(options) == -1 ||
-          this->setEnvVariable(options) == -1) {
-        return -1;
-      }
-      return 0;
-    }
-
-    virtual int setStdHandles(ACE_Process_Options &options) {
-      boost::filesystem::path p{this->programName_};
-
-      std::string tmp = p.filename().string();
-      boost::replace_all(tmp, " ", "_");
-      boost::replace_all(tmp, "-", "_");
-
-      auto stderr_path = p.parent_path();
-      stderr_path += boost::filesystem::path::preferred_separator;
-      stderr_path += tmp;
-
-      std::string stderr_name = stderr_path.string();
-      std::remove(stderr_name.c_str());
-
-      outputfd_.open(stderr_name,
-                     std::ios::in | std::ios::out | std::ios::trunc);
-      return options.set_handles(ACE_STDIN, ACE_STDOUT, outputfd_.handle());
-    }
-
-    virtual int setEnvVariable(ACE_Process_Options &options) {
-      return options.setenv("PRIVATE_VAR=/that/seems/to/be/it");
-    }
-    // Listing 2
-
-   private:
-   protected:
-    ~Manager() noexcept override = default;
-
-   private:
-    std::string programName_;
-    boost::iostreams::file_descriptor outputfd_;
-  };
-
-}  // namespace dunit.
-
-#endif  // GEODE_INTEGRATION_TEST_FW_SPAWN_H_