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.

Sign up for GitHub

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

Move libzmq dependent functions out of libflux-core into libzmqutil #3797

Merged
merged 12 commits into from
Jul 31, 2021
Merged

Move libzmq dependent functions out of libflux-core into libzmqutil #3797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4136383
job-manager: Fix comment typo
chu11 Jul 23, 2021
c505e14
broker: fix invalid tabbing
chu11 Jul 23, 2021
eb82078
libflux: allow msg create to take FLUX_MSGTYPE_ANY
chu11 Jul 12, 2021
f995e0f
libflux/message: create route id wrapper functions
chu11 Jul 29, 2021
4dbf230
libflux: rename msg_append_route()
chu11 Jul 29, 2021
ed4fce5
libflux: create internal message library
chu11 Jul 11, 2021
37474d3
libzmqutil: create new zmqutil internal library
chu11 Jul 18, 2021
d0db3eb
libutil: move ev_zmq to libzmqutil
chu11 Jul 23, 2021
091bbab
libflux: create reactor_private header file
chu11 Jul 23, 2021
850c479
libflux: move zmq reactor calls into libzmqutil
chu11 Jul 23, 2021
c0aed78
doc: Remove zmq watcher documentation
chu11 Jul 23, 2021
e4b64a4
testsuite: Add regression test for issue 3617
chu11 Jul 28, 2021
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 configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ AC_CONFIG_FILES( \
src/common/librlist/Makefile \
src/common/libczmqcontainers/Makefile \
src/common/libccan/Makefile \
src/common/libzmqutil/Makefile \
src/bindings/Makefile \
src/bindings/lua/Makefile \
src/bindings/python/Makefile \
Expand Down
2 changes: 0 additions & 2 deletions doc/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ MAN3_FILES_PRIMARY = \
man3/flux_reactor_create.3 \
man3/flux_fd_watcher_create.3 \
man3/flux_watcher_start.3 \
man3/flux_zmq_watcher_create.3 \
man3/flux_handle_watcher_create.3 \
man3/flux_timer_watcher_create.3 \
man3/flux_periodic_watcher_create.3 \
Expand Down Expand Up @@ -163,7 +162,6 @@ MAN3_FILES_SECONDARY = \
man3/flux_watcher_stop.3 \
man3/flux_watcher_destroy.3 \
man3/flux_watcher_next_wakeup.3 \
man3/flux_zmq_watcher_get_zsock.3 \
man3/flux_handle_watcher_get_flux.3 \
man3/flux_timer_watcher_reset.3 \
man3/flux_periodic_watcher_reset.3 \
Expand Down
2 changes: 0 additions & 2 deletions doc/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,8 +332,6 @@ def setup(app):
('man3/flux_watcher_start', 'flux_watcher_destroy', 'start/stop/destroy/query reactor watcher', [author], 3),
('man3/flux_watcher_start', 'flux_watcher_next_wakeup', 'start/stop/destroy/query reactor watcher', [author], 3),
('man3/flux_watcher_start', 'flux_watcher_start', 'start/stop/destroy/query reactor watcher', [author], 3),
('man3/flux_zmq_watcher_create', 'flux_zmq_watcher_get_zsock', 'create ZeroMQ watcher', [author], 3),
('man3/flux_zmq_watcher_create', 'flux_zmq_watcher_create', 'create ZeroMQ watcher', [author], 3),
('man3/idset_create', 'idset_create', 'Manipulate numerically sorted sets of non-negative integers', [author], 3),
('man3/idset_create', 'idset_destroy', 'Manipulate numerically sorted sets of non-negative integers', [author], 3),
('man3/idset_create', 'idset_set', 'Manipulate numerically sorted sets of non-negative integers', [author], 3),
Expand Down
3 changes: 1 addition & 2 deletions doc/man3/flux_reactor_create.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ SEE ALSO
========

flux_fd_watcher_create(3), flux_handle_watcher_create(3),
flux_zmq_watcher_create(3), flux_timer_watcher_create(3),
flux_watcher_start(3)
flux_timer_watcher_create(3), flux_watcher_start(3)

`libev home page <http://software.schmorp.de/pkg/libev.html>`__
87 changes: 0 additions & 87 deletions doc/man3/flux_zmq_watcher_create.rst
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion doc/man3/index.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ man3
flux_sync_create
flux_timer_watcher_create
flux_watcher_start
flux_zmq_watcher_create
idset_create
idset_encode
flux_jobtap_get_flux
Expand Down
2 changes: 2 additions & 0 deletions src/broker/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ libbroker_la_SOURCES = \
flux_broker_LDADD = \
$(builddir)/libbroker.la \
$(top_builddir)/src/common/libflux-core.la \
$(top_builddir)/src/common/libzmqutil/libzmqutil.la \
$(top_builddir)/src/common/libpmi/libpmi_client.la \
$(top_builddir)/src/common/libflux-internal.la \
$(top_builddir)/src/common/libflux-optparse.la \
Expand All @@ -89,6 +90,7 @@ test_ldadd = \
$(builddir)/libbroker.la \
$(top_builddir)/src/common/libtestutil/libtestutil.la \
$(top_builddir)/src/common/libflux-core.la \
$(top_builddir)/src/common/libzmqutil/libzmqutil.la \
$(top_builddir)/src/common/libpmi/libpmi_client.la \
$(top_builddir)/src/common/libflux-internal.la \
$(top_builddir)/src/common/libtap/libtap.la \
Expand Down
14 changes: 8 additions & 6 deletions src/broker/module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include <sys/syscall.h>
#endif
Copy link
Member

@garlick garlick Jul 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Commit message for c9096f7

This might be the commit that people will find in the history when they ask "why isn't libzmqutil part of libflux"? So it might be good to mention the FIPS startup penalty and its adverse effect on job throughput in the commit message problem statement, even though the issue reference does provide that background. Something like:

Problem: on systems configured for FIPS compliance, the cyrpto libs used by libzmq perform a self-test in the dso init function, which slows down library loading. When linking with libzmq is widespread, this significantly impairs job throughput.

Isolate 0MQ functions that depend on libzmq in a non-exported convenience library, to make it possible to minimize the number of components linked with libzmq.


#include "src/common/libzmqutil/msg_zsock.h"
#include "src/common/libzmqutil/reactor.h"
#include "src/common/libczmqcontainers/czmq_containers.h"
#include "src/common/libutil/log.h"
#include "src/common/libutil/iterators.h"
Expand Down Expand Up @@ -253,7 +255,7 @@ flux_msg_t *module_recvmsg (module_t *p)
int type;
struct flux_msg_cred cred;

if (!(msg = flux_msg_recvzsock (p->sock)))
if (!(msg = zmqutil_msg_recv (p->sock)))
goto error;
if (flux_msg_get_type (msg, &type) < 0)
goto error;
Expand Down Expand Up @@ -314,7 +316,7 @@ int module_sendmsg (module_t *p, const flux_msg_t *msg)
goto done;
if (flux_msg_route_push (cpy, p->modhash->uuid_str) < 0)
goto done;
if (flux_msg_sendzsock (p->sock, cpy) < 0)
if (zmqutil_msg_send (p->sock, cpy) < 0)
goto done;
break;
}
Expand All @@ -323,12 +325,12 @@ int module_sendmsg (module_t *p, const flux_msg_t *msg)
goto done;
if (flux_msg_route_delete_last (cpy) < 0)
goto done;
if (flux_msg_sendzsock (p->sock, cpy) < 0)
if (zmqutil_msg_send (p->sock, cpy) < 0)
goto done;
break;
}
default:
if (flux_msg_sendzsock (p->sock, msg) < 0)
if (zmqutil_msg_send (p->sock, msg) < 0)
goto done;
break;
}
Expand Down Expand Up @@ -612,13 +614,13 @@ module_t *module_add (modhash_t *mh, const char *path)
log_err ("zsock_bind inproc://%s", module_get_uuid (p));
goto cleanup;
}
if (!(p->broker_w = flux_zmq_watcher_create (
if (!(p->broker_w = zmqutil_watcher_create (
flux_get_reactor (p->modhash->broker_h),
p->sock,
FLUX_POLLIN,
module_cb,
p))) {
log_err ("flux_zmq_watcher_create");
log_err ("zmqutil_watcher_create");
goto cleanup;
}
/* Set creds for connection.
Expand Down
42 changes: 22 additions & 20 deletions src/broker/overlay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <jansson.h>
#include <uuid.h>

#include "src/common/libzmqutil/msg_zsock.h"
#include "src/common/libzmqutil/reactor.h"
#include "src/common/libczmqcontainers/czmq_containers.h"
#include "src/common/libutil/log.h"
#include "src/common/libutil/kary.h"
Expand Down Expand Up @@ -349,7 +351,7 @@ static int overlay_sendmsg_parent (struct overlay *ov, const flux_msg_t *msg)
errno = EHOSTUNREACH;
goto done;
}
rc = flux_msg_sendzsock (ov->parent.zsock, msg);
rc = zmqutil_msg_send (ov->parent.zsock, msg);
if (rc == 0)
ov->parent.lastsent = flux_reactor_now (ov->reactor);
done:
Expand Down Expand Up @@ -501,7 +503,7 @@ static int overlay_sendmsg_child (struct overlay *ov, const flux_msg_t *msg)
errno = EHOSTUNREACH;
goto done;
}
rc = flux_msg_sendzsock_ex (ov->bind_zsock, msg, true);
rc = zmqutil_msg_send_ex (ov->bind_zsock, msg, true);
done:
return rc;
}
Expand Down Expand Up @@ -569,7 +571,7 @@ static void child_cb (flux_reactor_t *r, flux_watcher_t *w,
struct child *child;
int status;

if (!(msg = flux_msg_recvzsock (ov->bind_zsock)))
if (!(msg = zmqutil_msg_recv (ov->bind_zsock)))
return;
if (flux_msg_get_type (msg, &type) < 0
|| !(sender = flux_msg_route_last (msg)))
Expand Down Expand Up @@ -630,7 +632,7 @@ static void parent_cb (flux_reactor_t *r, flux_watcher_t *w,
int type;
const char *topic = NULL;

if (!(msg = flux_msg_recvzsock (ov->parent.zsock)))
if (!(msg = zmqutil_msg_recv (ov->parent.zsock)))
return;
if (flux_msg_get_type (msg, &type) < 0) {
goto drop;
Expand Down Expand Up @@ -770,11 +772,11 @@ static int overlay_zap_init (struct overlay *ov)
log_err ("could not bind to %s", ZAP_ENDPOINT);
return -1;
}
if (!(ov->zap_w = flux_zmq_watcher_create (ov->reactor,
ov->zap,
FLUX_POLLIN,
overlay_zap_cb,
ov)))
if (!(ov->zap_w = zmqutil_watcher_create (ov->reactor,
ov->zap,
FLUX_POLLIN,
overlay_zap_cb,
ov)))
return -1;
flux_watcher_start (ov->zap_w);
return 0;
Expand Down Expand Up @@ -943,12 +945,12 @@ int overlay_connect (struct overlay *ov)
zsock_set_identity (ov->parent.zsock, ov->uuid);
if (zsock_connect (ov->parent.zsock, "%s", ov->parent.uri) < 0)
goto nomem;
if (!(ov->parent.w = flux_zmq_watcher_create (ov->reactor,
ov->parent.zsock,
FLUX_POLLIN,
parent_cb,
ov)))
return -1;
if (!(ov->parent.w = zmqutil_watcher_create (ov->reactor,
ov->parent.zsock,
FLUX_POLLIN,
parent_cb,
ov)))
return -1;
flux_watcher_start (ov->parent.w);
if (hello_request_send (ov, ov->rank, FLUX_CORE_VERSION_HEX) < 0)
return -1;
Expand Down Expand Up @@ -982,11 +984,11 @@ int overlay_bind (struct overlay *ov, const char *uri)
*/
if (!(ov->bind_uri = zsock_last_endpoint (ov->bind_zsock)))
return -1;
if (!(ov->bind_w = flux_zmq_watcher_create (ov->reactor,
ov->bind_zsock,
FLUX_POLLIN,
child_cb,
ov)))
if (!(ov->bind_w = zmqutil_watcher_create (ov->reactor,
ov->bind_zsock,
FLUX_POLLIN,
child_cb,
ov)))
return -1;
flux_watcher_start (ov->bind_w);
/* Ensure that ipc files are removed when the broker exits.
Expand Down
7 changes: 4 additions & 3 deletions src/broker/test/overlay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <czmq.h>

#include "src/common/libtap/tap.h"
#include "src/common/libzmqutil/msg_zsock.h"
#include "src/common/libczmqcontainers/czmq_containers.h"
#include "src/common/libtestutil/util.h"
#include "src/common/libutil/stdlog.h"
Expand Down Expand Up @@ -436,7 +437,7 @@ void trio (flux_t *h)
zsock_set_identity (zsock_none, "2");
ok (zsock_connect (zsock_none, "%s", parent_uri) == 0,
"none-2: zsock_connect %s (no security) works", parent_uri);
ok (flux_msg_sendzsock (zsock_none, msg) == 0,
ok (zmqutil_msg_send (zsock_none, msg) == 0,
"none-2: zsock_msg_sendzsock works");

/* 2) Curve, and correct server publc key, but client public key
Expand All @@ -453,8 +454,8 @@ void trio (flux_t *h)
zcert_destroy (&cert);
ok (zsock_connect (zsock_curve, "%s", parent_uri) == 0,
"curve-2: zsock_connect %s works", parent_uri);
ok (flux_msg_sendzsock (zsock_curve, msg) == 0,
"curve-2: flux_msg_sendzsock works");
ok (zmqutil_msg_send (zsock_curve, msg) == 0,
"curve-2: zmqutil_msg_send works");

/* Neither of the above attempts should have gotten a message through.
*/
Expand Down
5 changes: 3 additions & 2 deletions src/common/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ SUBDIRS = \
libhostlist \
librlist \
libczmqcontainers \
libccan
libccan \
libzmqutil

AM_CFLAGS = $(WARNING_CFLAGS) $(CODE_COVERAGE_CFLAGS)
AM_LDFLAGS = $(CODE_COVERAGE_LIBS)
Expand All @@ -34,6 +35,7 @@ fluxinclude_HEADERS = core.h schedutil.h
noinst_LTLIBRARIES = libflux-internal.la
libflux_internal_la_SOURCES =
libflux_internal_la_LIBADD = \
$(builddir)/libflux/libmessage.la \
$(builddir)/liblsd/liblsd.la \
$(builddir)/libccan/libccan.la \
$(builddir)/libutil/libutil.la \
Expand All @@ -47,7 +49,6 @@ libflux_internal_la_LIBADD = \
$(builddir)/libhostlist/libhostlist.la \
$(builddir)/libczmqcontainers/libczmqcontainers.la \
$(JANSSON_LIBS) \
$(ZMQ_LIBS) \
$(LIBUUID_LIBS) \
$(LIBPTHREAD) \
$(LIBDL) \
Expand Down
Loading