Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Use pipe to properly block clients when server hasn't entered event loop. #1541

Merged
merged 4 commits into from
Nov 10, 2016

Conversation

objmagic
Copy link
Contributor

@objmagic objmagic commented Nov 8, 2016

The previous way of using usleep cannot guarantee the clients are blocked when server hasn't entered event loop, which makesheron/common/tests/cpp/network:http_unittest flaky.

This PR switches to using pipe. Server will send a byte to pipe prior to entering event loop. Clients will wait(block) until the byte is read on the other side of the pipe. This makes sure that clients are properly blocked.

Test done via script

#!/bin/sh

for i in `seq 1 1000`;
do
    bazel test --config=darwin --no_cache_test_results heron/common/tests/cpp/network:http_unittest
    if [[ $? != 0 ]]; then
      exit 1
    fi
done

Thanks @congwang for suggestions.

server hasn't entered event loop.
@objmagic objmagic added this to the 0.14.5 milestone Nov 8, 2016
@objmagic objmagic self-assigned this Nov 8, 2016
::usleep(1000);
// choose an arbitrary number for ``sent'' as token
int recv, sent = 19583;
if (pipe(fds) < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use ::pipe().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved via 0238865


// block clients from reading from server using pipe
read(fds[0], &recv, sizeof(int));
if (recv != sent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to check the value, we only have one reader and one writer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved via 593f35a

@kramasamy
Copy link
Contributor

Good one! Thanks for fixing this.

@@ -35,7 +36,7 @@ static const sp_uint32 SERVER_PORT = 60000;

extern void start_http_client(sp_uint32 _port, sp_uint64 _requests, sp_uint32 _nkeys);

extern void start_http_server(sp_uint32 _port, sp_uint32 _nkeys);
extern void start_http_server(sp_uint32 _port, sp_uint32 _nkeys, int fd, int sent);
Copy link
Contributor

Choose a reason for hiding this comment

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

'sent' is no longer needed either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved via dc18567

@congwang
Copy link
Contributor

congwang commented Nov 9, 2016

Looks good to me.

@billonahill
Copy link
Contributor

Should fix #1544.

@objmagic objmagic merged commit 7aebdf6 into apache:master Nov 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants