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

Correct concurrency bugs when running tests, along with a bugfix & small warning cleanup #1683

Merged
merged 3 commits into from
Apr 26, 2022

Conversation

qkoziol
Copy link
Contributor

@qkoziol qkoziol commented Apr 24, 2022

The tests use an existing services port (3000), switch to a random ephemeral port # for each run, to avoid clashing with concurrently running tests on the same machine. Wait for the server to start and stop properly. Fix an arg parsing bug in the server.

@@ -1274,8 +1295,8 @@ create_mirroring_split_fapl(const char *_basename, struct mirrortest_filenames *
*/
mirror_conf.magic = H5FD_MIRROR_FAPL_MAGIC;
mirror_conf.version = H5FD_MIRROR_CURR_FAPL_T_VERSION;
mirror_conf.handshake_port = SERVER_HANDSHAKE_PORT;
if (HDstrncpy(mirror_conf.remote_ip, SERVER_IP_ADDRESS, H5FD_MIRROR_MAX_IP_LEN) == NULL) {
mirror_conf.handshake_port = opts->portno;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use port & IP from command line

* ----------------------------------------------------------------------------
*/
static int
confirm_server(struct mt_opts *opts)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait for the server to be available, since it was started as a background process in the test script.

@@ -22,7 +22,10 @@
nerrors=0

SERVER_VERBOSITY="--verbosity=1"
SERVER_PORT="--port=3000"
# Choose random ephemeral port number
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do not hardcode port #, especially one below 49152, which are allocated to existing services. Randomizing the port # helps avoid choosing the same port # as a concurrency running mirror test script, albeit imperfectly.

@@ -83,12 +86,15 @@ echo "Launching Mirror Server"
SERVER_ARGS="$SERVER_PORT $SERVER_VERBOSITY"
./mirror_server $SERVER_ARGS &

./mirror_vfd
./mirror_vfd $SERVER_PORT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test binary needs to know where the server landed.

nerrors=$?

echo "Stopping Mirror Server"
./mirror_server_stop $SERVER_PORT

# Wait for background server process to exit
wait
Copy link
Contributor Author

@qkoziol qkoziol Apr 24, 2022

Choose a reason for hiding this comment

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

Wait for server to finish running in the background, in case the test is running again very quickly after the previous one and the previous server hasn't exited yet.

@@ -46,11 +46,8 @@

#ifdef H5_HAVE_MIRROR_VFD

#define MAXBUF 2048 /* max buffer length. */
#define LISTENQ 80 /* max pending mirrorS requests */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused

@@ -211,8 +208,8 @@ parse_args(int argc, char **argv, struct op_args *args_out)
return -1;
}

/* Loop over arguments after program name and writer_path */
for (i = 2; i < argc; i++) {
/* Loop over arguments after program name */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently the "writer path" wasn't implemented?

/* Loop over arguments after program name and writer_path */
for (i = 2; i < argc; i++) {
/* Loop over arguments after program name */
for (i = 1; i < argc; i++) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correctly parse all command line args

@@ -536,10 +533,27 @@ handle_requests(struct server_run *run)
if (!HDstrncmp("SHUTDOWN", mybuf, 8)) {
/* Stop operation if told to stop */
mirror_log(run->loginfo, V_INFO, "received SHUTDOWN!", ret);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirm shutdown request

HDclose(connfd);
connfd = -1;
goto done;
} /* end if explicit "SHUTDOWN" directive */
if (!HDstrncmp("CONFIRM", mybuf, 7)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack check for server's existence

@@ -60,7 +60,7 @@ struct mshs_opts {
static void
usage(void)
{
HDprintf("mirror_server_halten_sie [options]\n"
HDprintf("mirror_server_stop [options]\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct name

@@ -157,6 +158,16 @@ send_shutdown(struct mshs_opts *opts)
return -1;
}

/* Read & verify response from port connection. */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait for server to reply before exiting this program

@qkoziol qkoziol merged commit 417ee13 into HDFGroup:develop Apr 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants