Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CBRD-25436] Creating monitoring thread and data structures for cub_server monitoring #5293

Merged
merged 31 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9216977
add data structures and worker thread
YeunjunLee Jul 1, 2024
5fa32d1
remove debugging logs
YeunjunLee Jul 2, 2024
53d83fd
indent
YeunjunLee Jul 2, 2024
1214fb9
indent cpp
YeunjunLee Jul 2, 2024
e35c2a8
remove debugging log in master_monitoring_list
YeunjunLee Jul 2, 2024
27144c4
remove whitespace
YeunjunLee Jul 2, 2024
29a41b4
make master_monitor class and put server_entry, master_monitor_list, …
YeunjunLee Jul 3, 2024
dc02a08
modify licence
YeunjunLee Jul 3, 2024
6c04ca7
indent
YeunjunLee Jul 4, 2024
93476d6
Improve class structure, Utilizaing unique ptr, Change names of membe…
YeunjunLee Jul 8, 2024
bd0e96b
remove undeleted code
YeunjunLee Jul 8, 2024
6973275
remove undeleted code
YeunjunLee Jul 8, 2024
419557b
remove undeleted code
YeunjunLee Jul 8, 2024
c507718
remove undeleted code
YeunjunLee Jul 8, 2024
5d7dc21
fix while loop
YeunjunLee Jul 8, 2024
33920c9
Remove unnessesary pointers in class
YeunjunLee Jul 8, 2024
503ede2
Remove whitespace
YeunjunLee Jul 8, 2024
72c60eb
Modify some comments
YeunjunLee Jul 8, 2024
96b0ebd
Add comments for member variables
YeunjunLee Jul 8, 2024
a0b20c4
Add delete_instance, Change m_thread_shutdown to volatile
YeunjunLee Jul 9, 2024
d3c8708
Align comments
YeunjunLee Jul 9, 2024
d842e7c
Make constructor of server_monitor as private
YeunjunLee Jul 9, 2024
e838efa
Remove m_mutex in server_monitor
YeunjunLee Jul 9, 2024
7e3f2df
Change singleton pattern from unique_ptr to static
YeunjunLee Jul 9, 2024
4d0a6ca
Change server_monitor from singleton pattern to global variable
YeunjunLee Jul 10, 2024
9e47e76
Disable formatting
YeunjunLee Jul 10, 2024
f46fe82
Remove master_monitor_list inner class
YeunjunLee Jul 11, 2024
a7be8ff
Change position of global variable
YeunjunLee Jul 11, 2024
daee3e1
Fix typo
YeunjunLee Jul 11, 2024
9cdbea3
Add assertion in destructor of server_monitor
YeunjunLee Jul 11, 2024
5b9f8cd
Change debugging message
YeunjunLee Jul 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ set(EXECUTABLE_SOURCES
${EXECUTABLES_DIR}/unloaddb.c
${EXECUTABLES_DIR}/util_common.c
${EXECUTABLES_DIR}/util_cs.c
${EXECUTABLES_DIR}/master_server_monitor.cpp
)

set(EXECUTABLE_HEADERS
Expand Down
17 changes: 17 additions & 0 deletions src/executables/master.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#include "message_catalog.h"
#include "dbi.h"
#include "util_func.h"
#include "master_server_monitor.hpp"

static void css_master_error (const char *error_string);
static int css_master_timeout (void);
Expand Down Expand Up @@ -104,6 +105,10 @@ static int css_check_master_socket_exception (fd_set * fd_var);
static void css_master_loop (void);
static void css_free_entry (SOCKET_QUEUE_ENTRY * entry_p);

// *INDENT-OFF*
std::unique_ptr < server_monitor > master_server_monitor = nullptr;
hornetmj marked this conversation as resolved.
Show resolved Hide resolved
hornetmj marked this conversation as resolved.
Show resolved Hide resolved
joohok marked this conversation as resolved.
Show resolved Hide resolved
joohok marked this conversation as resolved.
Show resolved Hide resolved
// *INDENT-ON*

#if !defined(WINDOWS)
static void css_daemon_start (void);
#endif
Expand Down Expand Up @@ -1223,6 +1228,18 @@ main (int argc, char **argv)
}
#endif

// TODO : When no non-HA server exists in HA environment, server_monitor should not be initialized.
// In this issue, server_monitor is initialized only when HA is disabled. Once all sub-tasks of
// CBRD-24741 are done, this condition will be removed. And server_monitor will be initialized wh
// en first non-HA server is started in HA environment. (server_monitor will be finalized when last
// non-HA server is stopped in HA environment.)
if (HA_DISABLED ())
{
// *INDENT-OFF*
master_server_monitor.reset(new server_monitor());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
master_server_monitor.reset(new server_monitor());
master_server_monitor.reset (new server_monitor ());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정하였습니다.

// *INDENT-ON*
}

conn = css_make_conn (css_Master_socket_fd[0]);
css_add_request_to_socket_queue (conn, false, NULL, css_Master_socket_fd[0], READ_WRITE, 0,
&css_Master_socket_anchor);
Expand Down
89 changes: 89 additions & 0 deletions src/executables/master_server_monitor.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
*
* Copyright 2016 CUBRID Corporation
*
* Licensed 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.
*
*/

//
// master_server_monitor.cpp - Server Revive monitoring module
//

#include "master_server_monitor.hpp"
#include <sstream>

#include "util_func.h"
hornetmj marked this conversation as resolved.
Show resolved Hide resolved

server_monitor::server_monitor ()
{
m_thread_shutdown = false;
m_monitor_list = std::make_unique<server_monitor_list>();
fprintf (stdout, "server_monitor_list is created. \n");

m_monitoring_thread = std::make_unique<std::thread> ([this]()
{
while (!m_thread_shutdown)
{
// TODO: select server_entry whose m_need_revive value is true. (Will be implemented in CBRD-25438 issue.)
}
});
fprintf (stdout, "server_monitor_thread is created. \n");
fflush (stdout);
hornetmj marked this conversation as resolved.
Show resolved Hide resolved
}

// In server_monitor destructor, it should guerentee that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// In server_monitor destructor, it should guerentee that
// In server_monitor destructor, it should guarantee that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정하였습니다.

// m_monitoring_thread is terminated before m_monitor_list is deleted.
server_monitor::~server_monitor ()
{
m_thread_shutdown = true;
if (m_monitoring_thread->joinable())
{
m_monitoring_thread->join();
fprintf (stdout, "server_monitor_thread is terminated. \n");
}
fprintf (stdout, "server_monitor_list is deleted. \n");
fflush (stdout);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
server_monitor::~server_monitor ()
{
m_thread_shutdown = true;
if (m_monitoring_thread->joinable())
{
m_monitoring_thread->join();
fprintf (stdout, "server_monitor_thread is terminated. \n");
}
fprintf (stdout, "server_monitor_list is deleted. \n");
fflush (stdout);
}
server_monitor::~server_monitor ()
{
m_thread_shutdown = true;
if (m_monitoring_thread->joinable())
{
m_monitoring_thread->join();
fprintf (stdout, "server_monitor_thread is terminated. \n");
}
assert (m_server_entry_list->size () == 0);
fprintf (stdout, "server_entry_list is deleted. \n");
fflush (stdout);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정하였습니다.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메세지 출력 시 server_monitor_list 아닌 server_entry_list로 제안드렸습니다. 제안된 내용을 그대로 반영하시려면, Commit suggestion 버튼을 이용하시면 됩니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server_entry_list 생성 / 제거 디버깅 메시지 둘 다 수정하였습니다.


server_monitor::server_entry::
server_entry (int pid, const char *server_name, const char *exec_path, char *args, CSS_CONN_ENTRY *conn)
: m_pid {pid}
, m_server_name {server_name}
, m_exec_path {exec_path}
, m_conn {conn}
joohok marked this conversation as resolved.
Show resolved Hide resolved
, m_last_revive_time {0, 0}
, m_need_revive {false}
{
if (args != nullptr)
{
proc_make_arg (args);
}
}

void
server_monitor::server_entry::proc_make_arg (char *args)
{
std::istringstream iss (args);
std::string tok;
while (iss >> tok)
{
m_argv.push_back (tok);
}
}

server_monitor::server_monitor_list::server_monitor_list()
{
m_unacceptable_proc_restart_timediff = prm_get_integer_value (PRM_ID_HA_UNACCEPTABLE_PROC_RESTART_TIMEDIFF_IN_MSECS);
m_max_process_start_confirm = prm_get_integer_value (PRM_ID_HA_MAX_PROCESS_START_CONFIRM);
}
joohok marked this conversation as resolved.
Show resolved Hide resolved
99 changes: 99 additions & 0 deletions src/executables/master_server_monitor.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
*
* Copyright 2016 CUBRID Corporation
*
* Licensed 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.
*
*/

/*
* master_server_monitor.hpp -
*/

joohok marked this conversation as resolved.
Show resolved Hide resolved
#ifndef _MASTER_SERVER_MONITOR_HPP_
#define _MASTER_SERVER_MONITOR_HPP_

#include <cstring>
#include <thread>
#include <algorithm>
#include <memory>
#include <mutex>
#include <time.h>
#include "heartbeat.h"
#include "system_parameter.h"
#include "connection_defs.h"
hornetmj marked this conversation as resolved.
Show resolved Hide resolved

class server_monitor
{
public:
class server_entry
hornetmj marked this conversation as resolved.
Show resolved Hide resolved
{
public:
server_entry (int pid, const char *server_name, const char *exec_path, char *args, CSS_CONN_ENTRY *conn);
~server_entry () {};

server_entry (const server_entry &) = delete;
server_entry (server_entry &&) = delete;

server_entry &operator = (const server_entry &) = delete;
server_entry &operator = (server_entry &&) = delete;

private:
void proc_make_arg (char *args);

const int m_pid; // process ID
const std::string m_server_name; // server name
const std::string m_exec_path; // executable path of server process
std::vector<std::string> m_argv; // arguments of server process
CSS_CONN_ENTRY *m_conn; // connection entry of server process
timeval m_last_revive_time; // latest revive time
bool m_need_revive; // need to revive (true if the server is abnormally terminated)
};

using server_entry_uptr_t = std::unique_ptr<server_monitor::server_entry>;

class server_monitor_list
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server_monitor_list를 별도의 내부 클래스로 유지하는 목적이 있을까요? server_entry를 위한 std::verctor<>는 server_monitor에 위치시켜도 충분할 것 같습니다. using을 이용해서 적당한 타입을 선언해 주면 될 것 같구요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후에 server_monitor_list 에 대한 탐색/삽입/제거/서버 revive 관련 함수들을 배치할 때, server_monitor class에 배치시키기 보다는 list에 대한 함수로서 배치하는 것이 더 보기 좋아보인다고 생각해서 별도의 클래스로 설계하였습니다.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::vector 자체가 list의 의미를 담고 있습니다. 다음 경우가 아니라면 크게 의미는 없지 않을까요?

  1. server_monitor가 여러개의 server_monitor_list를 만들어서 사용해야 하는 경우
  2. server_monitor_list 내부의 구조를 숨기고 싶은 경우
    • ex) server_monitor_list는 add_server_entry(), remove_server_entry() 인터페이스를 제공하며, server_monitor_list 내부에서 std::vector가 사용됨을 숨긴다. 이후 필요에 따라 std::vector는 다른 컨테이너나 사용자 정의 자료구조로 대체 가능하다.

그렇지 않다면, server_monitor는 std::vector에 접근하기 위해 server_monitor_list에 한번 더 접근해야 하기에 코드 복잡도가 증가합니다. 또한, class 타입 증가, 부모 class 라인수 증가 등의 이슈도 있습니다.

이후 진행될 sub-task에서 server_monitor_list 클래스 사용의 이점이 설명될 수 있다면, 이 리뷰는 해당 이슈에서 추가 논의해도 좋습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

server_monitor가 현재 위 두 경우에 해당하는 것 같지는 않습니다. 따라서 말씀 주신 대로, server_monior_list를 별개의 class로 설정하는 것 보다 server_monitor의 멤버로 직접 두는 것이 훨씬 간편할 것 같습니다.

해당 내용 반영하였고, 추가로 server_monitor_list 내부에 저장하고 있던 system parameter들도 사용 시 직접 가져와 사용하는 것이 클래스 구조를 간소화 하는데 더 좋을 것 같아서 제거하였습니다.

{
public:
server_monitor_list ();
~server_monitor_list () {};

server_monitor_list (const server_monitor_list &) = delete;
server_monitor_list (server_monitor_list &&) = delete;

server_monitor_list &operator = (const server_monitor_list &) = delete;
server_monitor_list &operator = (server_monitor_list &&) = delete;

private:
hornetmj marked this conversation as resolved.
Show resolved Hide resolved
std::vector <server_entry_uptr_t> server_entry_list; // list of server entries
hornetmj marked this conversation as resolved.
Show resolved Hide resolved
int m_unacceptable_proc_restart_timediff; // unacceptable time difference between process restart
int m_max_process_start_confirm; // Maximum number of process restart confirmations
};

server_monitor ();
~server_monitor ();

private:
server_monitor (const server_monitor &) = delete;
server_monitor (server_monitor &&) = delete;

server_monitor &operator = (const server_monitor &) = delete;
server_monitor &operator = (server_monitor &&) = delete;

hornetmj marked this conversation as resolved.
Show resolved Hide resolved
std::unique_ptr<std::thread> m_monitoring_thread; // monitoring thread
std::unique_ptr<server_monitor_list> m_monitor_list; // list of server entries
volatile bool m_thread_shutdown; // flag to shutdown monitoring thread
};

extern std::unique_ptr<server_monitor> master_server_monitor;
joohok marked this conversation as resolved.
Show resolved Hide resolved
#endif
Loading