Skip to content

Commit 85500df

Browse files
samuelkgutierrezsamuelkgutierrez
authored
Simplify RMI server architecture. (#302)
* Reduce the number of threads used in the RMI server architecture. They seem unnecessary and confusing to me. Signed-off-by: Samuel K. Gutierrez <[email protected]>
1 parent dbfeaf4 commit 85500df

File tree

7 files changed

+51
-268
lines changed

7 files changed

+51
-268
lines changed

‎include/quo-vadis-pthread.h

+9-9
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@ extern "C" {
3939
// - explicit: threads are placed according to a list of OS proc IDs (required)
4040
// TODO(skg) Do we need all of these synonyms?
4141
typedef enum {
42-
QV_POLICY_PACKED = 1,
43-
QV_POLICY_COMPACT = 1,
44-
QV_POLICY_CLOSE = 1,
45-
QV_POLICY_SPREAD = 2,
46-
QV_POLICY_DISTRIBUTE = 3,
47-
QV_POLICY_ALTERNATE = 3,
48-
QV_POLICY_CORESFIRST = 3,
49-
QV_POLICY_SCATTER = 4,
50-
QV_POLICY_CHOOSE = 5
42+
QV_POLICY_PACKED = 1,
43+
QV_POLICY_COMPACT = 1,
44+
QV_POLICY_CLOSE = 1,
45+
QV_POLICY_SPREAD = 2,
46+
QV_POLICY_DISTRIBUTE = 3,
47+
QV_POLICY_ALTERNATE = 3,
48+
QV_POLICY_CORESFIRST = 3,
49+
QV_POLICY_SCATTER = 4,
50+
QV_POLICY_CHOOSE = 5
5151
} qv_pthread_placement_t;
5252

5353
/**

‎src/quo-vadisd.cc

+2-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*- */
22
/*
3-
* Copyright (c) 2020-2024 Triad National Security, LLC
3+
* Copyright (c) 2020-2025 Triad National Security, LLC
44
* All rights reserved.
55
*
66
* Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -132,8 +132,7 @@ rmi_start(
132132

133133
cstr_t ers = nullptr;
134134

135-
static const bool blocks = true;
136-
int rc = qvi_rmi_server_start(ctx.rmi, blocks);
135+
int rc = qvi_rmi_server_start(ctx.rmi);
137136
if (rc != QV_SUCCESS) {
138137
ers = "qvi_rmi_server_start() failed";
139138
}

‎src/qvi-rmi.cc

+18-87
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,6 @@ do { \
5353
qvi_log_warn("{} with errno={} ({})", ers, (erno), qvi_strerr((erno))); \
5454
} while (0)
5555

56-
static constexpr cstr_t ZINPROC_ADDR = "inproc://qvi-rmi-workers";
57-
5856
typedef enum qvi_rpc_funid_e {
5957
FID_INVALID = 0,
6058
FID_SERVER_SHUTDOWN,
@@ -85,11 +83,6 @@ typedef int (*qvi_rpc_fun_ptr_t)(
8583
qvi_bbuff **
8684
);
8785

88-
static void
89-
send_server_shutdown_msg(
90-
qvi_rmi_server_t *server
91-
);
92-
9386
static void
9487
zsocket_close(
9588
void **sock
@@ -129,10 +122,6 @@ struct qvi_rmi_server_s {
129122
void *zctx = nullptr;
130123
/** Loopback socket for managerial messages. */
131124
void *zlo = nullptr;
132-
/** The worker thread. */
133-
pthread_t worker_thread;
134-
/** Flag indicating if main thread blocks for workers to complete. */
135-
bool blocks = false;
136125
/** Constructor. */
137126
qvi_rmi_server_s(void)
138127
{
@@ -145,14 +134,10 @@ struct qvi_rmi_server_s {
145134
/** Destructor. */
146135
~qvi_rmi_server_s(void)
147136
{
148-
send_server_shutdown_msg(this);
149137
zsocket_close(&zlo);
150138
zctx_destroy(&zctx);
151139
unlink(config.hwtopo_path.c_str());
152140
qvi_delete(&hwpool);
153-
if (!blocks) {
154-
pthread_join(worker_thread, nullptr);
155-
}
156141
}
157142
};
158143

@@ -772,16 +757,15 @@ server_rpc_dispatch(
772757
return (shutdown ? QV_SUCCESS_SHUTDOWN : rc);
773758
}
774759

775-
static void *
760+
static int
776761
server_go(
777-
void *data
762+
qvi_rmi_server_t *server
778763
) {
779-
qvi_rmi_server_t *const server = (qvi_rmi_server_t *)data;
780764

781-
void *zworksock = zsocket_create_and_connect(
782-
server->zctx, ZMQ_REP, ZINPROC_ADDR
765+
void *zworksock = zsocket_create_and_bind(
766+
server->zctx, ZMQ_REP, server->config.url.c_str()
783767
);
784-
if (qvi_unlikely(!zworksock)) return nullptr;
768+
if (qvi_unlikely(!zworksock)) return QV_ERR_SYS;
785769

786770
int rc, bsent;
787771
volatile int bsentt = 0;
@@ -807,15 +791,7 @@ server_go(
807791
if (qvi_unlikely(rc != QV_SUCCESS && rc != QV_SUCCESS_SHUTDOWN)) {
808792
qvi_log_error("RX/TX loop exited with rc={} ({})", rc, qv_strerr(rc));
809793
}
810-
return nullptr;
811-
}
812-
813-
static void
814-
send_server_shutdown_msg(
815-
qvi_rmi_server_t *server
816-
) {
817-
(void)rpc_req(server->zlo, FID_SERVER_SHUTDOWN, QVI_BBUFF_RMI_ZERO_MSG);
818-
(void)rpc_rep(server->zlo, QVI_BBUFF_RMI_ZERO_MSG);
794+
return QV_SUCCESS;
819795
}
820796

821797
int
@@ -840,49 +816,9 @@ server_populate_base_hwpool(
840816
return server->hwpool->initialize(hwloc, cpuset);
841817
}
842818

843-
static void *
844-
server_start_threads(
845-
void *data
846-
) {
847-
qvi_rmi_server_t *server = (qvi_rmi_server_t *)data;
848-
849-
void *clients = zsocket_create_and_bind(
850-
server->zctx, ZMQ_ROUTER, server->config.url.c_str()
851-
);
852-
if (qvi_unlikely(!clients)) {
853-
cstr_t ers = "zsocket_create_and_bind() failed";
854-
qvi_log_error("{}", ers);
855-
return nullptr;
856-
}
857-
858-
void *workers = zsocket_create_and_bind(
859-
server->zctx, ZMQ_DEALER, ZINPROC_ADDR
860-
);
861-
if (qvi_unlikely(!workers)) {
862-
cstr_t ers = "zsocket_create_and_bind() failed";
863-
qvi_log_error("{}", ers);
864-
return nullptr;
865-
}
866-
867-
pthread_t worker;
868-
int rc = pthread_create(&worker, nullptr, server_go, server);
869-
if (qvi_unlikely(rc != 0)) {
870-
cstr_t ers = "pthread_create() failed";
871-
qvi_log_error("{} with rc={} ({})", ers, rc, qvi_strerr(rc));
872-
}
873-
// The zmq_proxy() function always returns -1 and errno set to ETERM.
874-
zmq_proxy(clients, workers, nullptr);
875-
pthread_join(worker, nullptr);
876-
zsocket_close(&workers);
877-
878-
zsocket_close(&clients);
879-
return nullptr;
880-
}
881-
882819
int
883820
qvi_rmi_server_start(
884-
qvi_rmi_server_t *server,
885-
bool block
821+
qvi_rmi_server_t *server
886822
) {
887823
// First populate the base hardware resource pool.
888824
int qvrc = server_populate_base_hwpool(server);
@@ -891,23 +827,9 @@ qvi_rmi_server_start(
891827
server->zlo = zsocket_create_and_connect(
892828
server->zctx, ZMQ_REQ, server->config.url.c_str()
893829
);
894-
if (qvi_unlikely(!server->zlo)) return QV_ERR_RPC;
895-
896-
const int rc = pthread_create(
897-
&server->worker_thread, nullptr,
898-
server_start_threads, server
899-
);
900-
if (qvi_unlikely(rc != 0)) {
901-
cstr_t ers = "pthread_create() failed";
902-
qvi_log_error("{} with rc={} ({})", ers, rc, qvi_strerr(rc));
903-
qvrc = QV_ERR_SYS;
904-
}
830+
if (qvi_unlikely(!server->zlo)) return QV_ERR_SYS;
905831

906-
if (block && qvrc == QV_SUCCESS) {
907-
server->blocks = true;
908-
pthread_join(server->worker_thread, nullptr);
909-
}
910-
return qvrc;
832+
return server_go(server);
911833
}
912834

913835
static int
@@ -1096,6 +1018,15 @@ qvi_rmi_get_cpuset_for_nobjs(
10961018
);
10971019
}
10981020

1021+
int
1022+
qvi_rmi_send_shutdown_message(
1023+
qvi_rmi_client_t *client
1024+
) {
1025+
return rpc_req(
1026+
client->zsock, FID_SERVER_SHUTDOWN
1027+
);
1028+
}
1029+
10991030
/*
11001031
* vim: ft=cpp ts=4 sts=4 sw=4 expandtab
11011032
*/

‎src/qvi-rmi.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,7 @@ qvi_rmi_server_config(
8181
*/
8282
int
8383
qvi_rmi_server_start(
84-
qvi_rmi_server_t *server,
85-
bool block
84+
qvi_rmi_server_t *server
8685
);
8786

8887
/**
@@ -183,6 +182,11 @@ qvi_rmi_get_cpuset_for_nobjs(
183182
hwloc_cpuset_t *result
184183
);
185184

185+
int
186+
qvi_rmi_send_shutdown_message(
187+
qvi_rmi_client_t *client
188+
);
189+
186190
#ifdef __cplusplus
187191
}
188192
#endif

‎tests/internal/CMakeLists.txt

+3-26
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*-
22
#
3-
# Copyright (c) 2020-2024 Triad National Security, LLC
3+
# Copyright (c) 2020-2025 Triad National Security, LLC
44
# All rights reserved.
55
#
66
# Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC
@@ -31,26 +31,6 @@ add_test(
3131
test-hwloc
3232
)
3333

34-
################################################################################
35-
################################################################################
36-
add_executable(
37-
test-rmi-server
38-
test-rmi-server.cc
39-
)
40-
41-
target_link_libraries(
42-
test-rmi-server
43-
quo-vadis
44-
)
45-
46-
add_test(
47-
NAME
48-
rmi-server
49-
COMMAND
50-
bash -c "export URL=\"tcp://127.0.0.1:55995\" && \
51-
${CMAKE_CURRENT_BINARY_DIR}/test-rmi-server $URL"
52-
)
53-
5434
################################################################################
5535
################################################################################
5636
add_executable(
@@ -68,12 +48,10 @@ add_test(
6848
rmi
6949
COMMAND
7050
bash -c "export URL=\"tcp://127.0.0.1:55995\" && \
71-
( ${CMAKE_SOURCE_DIR}/tests/exec-timeout.sh \
72-
\"${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -s\" 5 & ) && \
73-
( ${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -c & ) && \
51+
( ${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -s & ) && \
7452
( ${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -c & ) && \
7553
( ${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -c & ) && \
76-
${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -c"
54+
${CMAKE_CURRENT_BINARY_DIR}/test-rmi $URL -cc"
7755
)
7856

7957
# Use the C linker to test for C/C++ linkage problems.
@@ -87,7 +65,6 @@ set_target_properties(
8765
# Set core test properties.
8866
set_tests_properties(
8967
hwloc
90-
rmi-server
9168
rmi
9269
PROPERTIES
9370
TIMEOUT 60

0 commit comments

Comments
 (0)