Merge lp:~stewart/galera/bug907623 into lp:galera/2.x

[Stewart Smith]

This branch enables the test suite to be run multiple times concurrently on the same machine.

Prior to this branch being merged, if you simply built Galera by using "scons" or build script in two completely different directories on the same machine, you could get test failures due to the test suite trying to open the same TCP port.

This branch changes the test suite to instead of having hard coded port numbers, to use Operating System assigned ones. This means you can have as many concurrent builds on a single machine as you have free TCP ports.

You get an OS assigned port number simply by binding to port 0 instead of 10001 or some such number. You then just ask the OS for what port number it gave you.

The changes are pretty minor to get there, and it *really* helps when you're running Continuous Integration.

[Alex Yurchenko (ayurchen)]

gu::Network is deprecated in favour of asio and should be removed from the code.

review: Disapprove

[Stewart Smith (stewart)]

I do not think that is a valid reason to reject this. This patch is a simple fix for a current problem, it is not a refactor of existing code (and that should be a separate patch). Rewriting large swaths of code just so that you can compile simultaneously doesn't really seem like the right way to do it - and certainly makes it harder to test any refactored code.

I really hope that you reconsider this one, as without a patch like this it makes it really hard for us to do further build and test.

[Alex Yurchenko (ayurchen)]

I have disabled gu::Network unit tests, so they should not interfer with the build anymore. Is there still a problem?

If you find that disabling unit tests is not enough and you still wish us to incorporate this patch, please explicitly assign the copyright to Codership Oy. (Which I'm not sure how to do with LP, honestly. Perhaps in the comments?)

review: Needs Resubmitting

[Stewart Smith (stewart)]

It looks like check_util is still run, which also deals with a TCP port. So it probably is a good idea to merge this branch.

I'm not sure what you need for copyright assignment - but I hereby assign Codership Oy all rights to the code in this merge request.

[Alex Yurchenko (ayurchen)]

Yes, our bad. We merged the relevant part of the patch in revno 107, please try if that works.

review: Approve

=== modified file 'galerautils/src/gu_network.cpp'
--- galerautils/src/gu_network.cpp	2011-02-01 19:13:25 +0000
+++ galerautils/src/gu_network.cpp	2012-01-04 05:59:23 +0000
@@ -441,8 +441,21 @@
     }
 
     set_opt(this, ai, ::get_opt(uri));
-    listener_ai = new Addrinfo(ai);
+
+    Sockaddr local_sa(sa);
+    struct sockaddr *servaddr= &(local_sa.get_sockaddr());
+    socklen_t sock_len= local_sa.get_sockaddr_len();
+
+    if (::getsockname(fd, servaddr, &sock_len) == -1)
+    {
+      set_state(S_FAILED, errno);
+      gu_throw_error(errno);
+    }
+
+    listener_ai = new Addrinfo(ai, local_sa);
     set_state(S_LISTENING);
+
+    local_addr = listener_ai->to_string();
 }
 
 gu::net::Socket* gu::net::Socket::accept()

=== modified file 'galerautils/tests/gu_net_test.cpp'
--- galerautils/tests/gu_net_test.cpp	2011-02-01 19:13:25 +0000
+++ galerautils/tests/gu_net_test.cpp	2012-01-04 05:59:23 +0000
@@ -197,7 +197,7 @@
 {
     log_info << "START";
     Network net;
-    Socket* listener = net.listen("tcp://localhost:2112");
+    Socket* listener = net.listen("tcp://localhost:0");
     listener->close();
     delete listener;
 }
@@ -301,16 +301,19 @@
     gu_log_max_level = GU_LOG_DEBUG;
     log_info << "START";
     Network* net = new Network;
-    Socket* listener = net->listen("tcp://localhost:2112");
+    Socket* listener = net->listen("tcp://localhost:0");
 
     log_info << "listener " << listener->get_local_addr();
 
+    std::string uri_str;
+    uri_str= listener->get_local_addr();
+
     listener_thd_args args = {net, 2, 0, 0};
     pthread_t th;
     pthread_create(&th, 0, &listener_thd, &args);
 
     Network* net2 = new Network;
-    Socket* conn = net2->connect("tcp://localhost:2112");
+    Socket* conn = net2->connect(uri_str);
 
     fail_unless(conn != 0);
     fail_unless(conn->get_state() == Socket::S_CONNECTED);
@@ -318,7 +321,7 @@
     log_info << "connected remote " << conn->get_remote_addr();
     log_info << "connected local " << conn->get_local_addr();
 
-    Socket* conn2 = net2->connect("tcp://localhost:2112");
+    Socket* conn2 = net2->connect(uri_str);
     fail_unless(conn2 != 0);
     fail_unless(conn2->get_state() == Socket::S_CONNECTED);
 
@@ -357,18 +360,21 @@
     }
 
     Network* net = new Network;
-    Socket* listener = net->listen("tcp://localhost:2112");
+    Socket* listener = net->listen("tcp://localhost:0");
+
+    std::string uri_str(listener->get_local_addr());
+
     listener_thd_args args = {net, 2, buf, bufsize};
     pthread_t th;
     pthread_create(&th, 0, &listener_thd, &args);
 
     Network* net2 = new Network;
-    Socket* conn = net2->connect("tcp://localhost:2112");
+    Socket* conn = net2->connect(uri_str);
 
     fail_unless(conn != 0);
     fail_unless(conn->get_state() == Socket::S_CONNECTED);
 
-    Socket* conn2 = net2->connect("tcp://localhost:2112");
+    Socket* conn2 = net2->connect(uri_str);
     fail_unless(conn2 != 0);
     fail_unless(conn2->get_state() == Socket::S_CONNECTED);
 
@@ -486,7 +492,8 @@
 }
 END_TEST
 
-static void make_connections(Network& net, 
+static void make_connections(std::string uri_str,
+			     Network& net,
                              vector<Socket*>& cl,
                              vector<Socket*>& sr,
                              size_t n)
@@ -495,7 +502,7 @@
     sr.resize(n);
     for (size_t i = 0; i < n; ++i)
     {
-        cl[i] = net.connect("tcp://localhost:2112?socket.non_blocking=1");
+        cl[i] = net.connect(uri_str);
     }
     size_t sr_cnt = 0;
     size_t cl_cnt = 0;
@@ -555,12 +562,16 @@
     log_info << "START";
     Network net;
 
-    Socket* listener = net.listen("tcp://localhost:2112?socket.non_blocking=1");
+    Socket* listener = net.listen("tcp://localhost:0?socket.non_blocking=1");
+    std::string uri_str(listener->get_local_addr());
+
+    uri_str.append("?socket.non_blocking=1");
+
     log_info << "listener: " << *listener;
     vector<Socket*> cl;
     vector<Socket*> sr;
     gu_log_max_level = GU_LOG_DEBUG;
-    make_connections(net, cl, sr, 3);
+    make_connections(uri_str, net, cl, sr, 3);
 
     close_connections(net, cl, sr);
 
@@ -666,6 +677,8 @@
 
 public:
 
+    std::string get_url() { return listener->get_local_addr(); }
+
     void connect(const string& url)
     {
         send_sock = net.connect(url);
@@ -782,8 +795,12 @@
 START_TEST(test_net_consumer)
 {
     log_info << "START";
-    string url("tcp://localhost:2112?socket.non_blocking=1");
+    string url("tcp://localhost:0?socket.non_blocking=1");
     NetConsumer cons(url);
+
+    url= cons.get_url();
+    url.append("?socket.non_blocking=1");
+
     cons.connect(url);
 
     cons.start();
@@ -853,8 +870,12 @@
 START_TEST(test_net_consumer_nto1)
 {
     log_info << "START";
-    string url("tcp://localhost:2112?socket.non_blocking=1");
+    string url("tcp://localhost:0?socket.non_blocking=1");
     NetConsumer cons(url);
+
+    url= cons.get_url();
+    url.append("?socket.non_blocking=1");
+
     cons.connect(url);
 
     cons.start();

=== modified file 'gcomm/test/check_util.cpp'
--- gcomm/test/check_util.cpp	2011-02-01 19:13:25 +0000
+++ gcomm/test/check_util.cpp	2012-01-04 05:59:23 +0000
@@ -58,10 +58,11 @@
 {
     gu::Config conf;
     AsioProtonet pn(conf);
-    const string uri_str("tcp://localhost:10001");
+    string uri_str("tcp://localhost:0");
 
     Acceptor* acc = pn.acceptor(uri_str);
     acc->listen(uri_str);
+    uri_str = acc->listen_addr();
 
     SocketPtr cl = pn.socket(uri_str);
     cl->connect(uri_str);