Patchwork runtests: check ports on IPv6 address

login
register
mail settings
Submitter Jun Wu
Date Feb. 9, 2017, 1:59 p.m.
Message ID <93e23f7b87a4ab456053.1486648775@localhost.localdomain>
Download mbox | patch
Permalink /patch/18361/
State Accepted
Headers show

Comments

Jun Wu - Feb. 9, 2017, 1:59 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1486648674 28800
#      Thu Feb 09 05:57:54 2017 -0800
# Node ID 93e23f7b87a4ab456053b6ba573615be16c6c4b0
# Parent  a68510b69f413545722c086eaeb840dd5e8305b4
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r 93e23f7b87a4
runtests: check ports on IPv6 address

Previously, checkportisavailable only checks ports on the IPv4 address. This
patch makes it check IPv6 as well. It'll be useful if "localhost" does not
have an IPv4 address, or its IPv4 address does not exist somehow.
Augie Fackler - Feb. 10, 2017, 2:50 a.m.
On Thu, Feb 09, 2017 at 05:59:35AM -0800, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1486648674 28800
> #      Thu Feb 09 05:57:54 2017 -0800
> # Node ID 93e23f7b87a4ab456053b6ba573615be16c6c4b0
> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 93e23f7b87a4
> runtests: check ports on IPv6 address

Queued, thanks.

> 
> Previously, checkportisavailable only checks ports on the IPv4 address. This
> patch makes it check IPv6 as well. It'll be useful if "localhost" does not
> have an IPv4 address, or its IPv4 address does not exist somehow.
> 
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -115,13 +115,17 @@ wifexited = getattr(os, "WIFEXITED", lam
> def checkportisavailable(port):
>     """return true if a port seems free to bind on localhost"""
> -    try:
> -        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> -        s.bind(('localhost', port))
> -        s.close()
> -        return True
> -    except socket.error as exc:
> -        if not exc.errno == errno.EADDRINUSE:
> -            raise
> -        return False
> +    families = [getattr(socket, i, None)
> +                for i in ('AF_INET', 'AF_INET6')
> +                if getattr(socket, i, None) is not None]
> +    for family in families:
> +        try:
> +            s = socket.socket(family, socket.SOCK_STREAM)
> +            s.bind(('localhost', port))
> +            s.close()
> +            return True
> +        except socket.error as exc:
> +            if exc.errno not in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
> +                raise
> +    return False
> 
> closefds = os.name == 'posix'
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Feb. 16, 2017, 4:12 a.m.
> On Feb 9, 2017, at 8:59 AM, Jun Wu <quark@fb.com> wrote:
> 
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1486648674 28800
> #      Thu Feb 09 05:57:54 2017 -0800
> # Node ID 93e23f7b87a4ab456053b6ba573615be16c6c4b0
> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 93e23f7b87a4
> runtests: check ports on IPv6 address

This has made test-bundle2-remote-changegroup.t flaky when run in parallel with other tests. I was able to bisect with this:

cd tests && python run-tests.py -j 100 --runs-per-test 400 test-bundle2-remote-changegroup.t

fails in about 70 out of the 400 runs. I also see it periodically when running the tests with -j120 on gcc112 from the gcc compile farm, which is a POWER8 machine with 160 hardware threads. I suspect you can reproduce it with a lower -j if you run the test enough times, but I’m up later than I should be already, so I didn’t bother logging in to a smaller linux machine to confirm.

Can you take a look?

Thanks!
Augie

> Previously, checkportisavailable only checks ports on the IPv4 address. This
> patch makes it check IPv6 as well. It'll be useful if "localhost" does not
> have an IPv4 address, or its IPv4 address does not exist somehow.
> 
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -115,13 +115,17 @@ wifexited = getattr(os, "WIFEXITED", lam
> def checkportisavailable(port):
>     """return true if a port seems free to bind on localhost"""
> -    try:
> -        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> -        s.bind(('localhost', port))
> -        s.close()
> -        return True
> -    except socket.error as exc:
> -        if not exc.errno == errno.EADDRINUSE:
> -            raise
> -        return False
> +    families = [getattr(socket, i, None)
> +                for i in ('AF_INET', 'AF_INET6')
> +                if getattr(socket, i, None) is not None]
> +    for family in families:
> +        try:
> +            s = socket.socket(family, socket.SOCK_STREAM)
> +            s.bind(('localhost', port))
> +            s.close()
> +            return True
> +        except socket.error as exc:
> +            if exc.errno not in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
> +                raise
> +    return False
> 
> closefds = os.name == 'posix'
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
via Mercurial-devel - Feb. 16, 2017, 4:23 a.m.
On Wed, Feb 15, 2017 at 8:12 PM, Augie Fackler <raf@durin42.com> wrote:
>
>> On Feb 9, 2017, at 8:59 AM, Jun Wu <quark@fb.com> wrote:
>>
>> # HG changeset patch
>> # User Jun Wu <quark@fb.com>
>> # Date 1486648674 28800
>> #      Thu Feb 09 05:57:54 2017 -0800
>> # Node ID 93e23f7b87a4ab456053b6ba573615be16c6c4b0
>> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
>> # Available At https://bitbucket.org/quark-zju/hg-draft
>> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 93e23f7b87a4
>> runtests: check ports on IPv6 address
>
> This has made test-bundle2-remote-changegroup.t flaky when run in parallel with other tests. I was able to bisect with this:
>
> cd tests && python run-tests.py -j 100 --runs-per-test 400 test-bundle2-remote-changegroup.t
>
> fails in about 70 out of the 400 runs. I also see it periodically when running the tests with -j120 on gcc112 from the gcc compile farm, which is a POWER8 machine with 160 hardware threads. I suspect you can reproduce it with a lower -j if you run the test enough times, but I’m up later than I should be already, so I didn’t bother logging in to a smaller linux machine to confirm.
>
> Can you take a look?

Heh, I reported the flakiness on #mercurial earlier today. That's why
Jun sent a series of patches maybe 2 hours ago that you then queued.
Have you tested with those patches applied? I have not had time to
look at his patches, but maybe he should have explained their
motivation better if this is indeed the same problem that I reported.

>
> Thanks!
> Augie
>
>> Previously, checkportisavailable only checks ports on the IPv4 address. This
>> patch makes it check IPv6 as well. It'll be useful if "localhost" does not
>> have an IPv4 address, or its IPv4 address does not exist somehow.
>>
>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>> --- a/tests/run-tests.py
>> +++ b/tests/run-tests.py
>> @@ -115,13 +115,17 @@ wifexited = getattr(os, "WIFEXITED", lam
>> def checkportisavailable(port):
>>     """return true if a port seems free to bind on localhost"""
>> -    try:
>> -        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>> -        s.bind(('localhost', port))
>> -        s.close()
>> -        return True
>> -    except socket.error as exc:
>> -        if not exc.errno == errno.EADDRINUSE:
>> -            raise
>> -        return False
>> +    families = [getattr(socket, i, None)
>> +                for i in ('AF_INET', 'AF_INET6')
>> +                if getattr(socket, i, None) is not None]
>> +    for family in families:
>> +        try:
>> +            s = socket.socket(family, socket.SOCK_STREAM)
>> +            s.bind(('localhost', port))
>> +            s.close()
>> +            return True
>> +        except socket.error as exc:
>> +            if exc.errno not in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
>> +                raise
>> +    return False
>>
>> closefds = os.name == 'posix'
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
via Mercurial-devel - Feb. 16, 2017, 4:24 a.m.
On Wed, Feb 15, 2017 at 8:23 PM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
> On Wed, Feb 15, 2017 at 8:12 PM, Augie Fackler <raf@durin42.com> wrote:
>>
>>> On Feb 9, 2017, at 8:59 AM, Jun Wu <quark@fb.com> wrote:
>>>
>>> # HG changeset patch
>>> # User Jun Wu <quark@fb.com>
>>> # Date 1486648674 28800
>>> #      Thu Feb 09 05:57:54 2017 -0800
>>> # Node ID 93e23f7b87a4ab456053b6ba573615be16c6c4b0
>>> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
>>> # Available At https://bitbucket.org/quark-zju/hg-draft
>>> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 93e23f7b87a4
>>> runtests: check ports on IPv6 address
>>
>> This has made test-bundle2-remote-changegroup.t flaky when run in parallel with other tests. I was able to bisect with this:
>>
>> cd tests && python run-tests.py -j 100 --runs-per-test 400 test-bundle2-remote-changegroup.t
>>
>> fails in about 70 out of the 400 runs. I also see it periodically when running the tests with -j120 on gcc112 from the gcc compile farm, which is a POWER8 machine with 160 hardware threads. I suspect you can reproduce it with a lower -j if you run the test enough times, but I’m up later than I should be already, so I didn’t bother logging in to a smaller linux machine to confirm.
>>
>> Can you take a look?
>
> Heh, I reported the flakiness on #mercurial earlier today. That's why
> Jun sent a series of patches maybe 2 hours ago that you then queued.
> Have you tested with those patches applied? I have not had time to
> look at his patches, but maybe he should have explained their
> motivation better if this is indeed the same problem that I reported.

Or maybe I just incorrectly assumed that the problems I reported with
ports not being available had the same root cause as the flaky
test-bundle2-remote-changegroup.t?

>
>>
>> Thanks!
>> Augie
>>
>>> Previously, checkportisavailable only checks ports on the IPv4 address. This
>>> patch makes it check IPv6 as well. It'll be useful if "localhost" does not
>>> have an IPv4 address, or its IPv4 address does not exist somehow.
>>>
>>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>>> --- a/tests/run-tests.py
>>> +++ b/tests/run-tests.py
>>> @@ -115,13 +115,17 @@ wifexited = getattr(os, "WIFEXITED", lam
>>> def checkportisavailable(port):
>>>     """return true if a port seems free to bind on localhost"""
>>> -    try:
>>> -        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>>> -        s.bind(('localhost', port))
>>> -        s.close()
>>> -        return True
>>> -    except socket.error as exc:
>>> -        if not exc.errno == errno.EADDRINUSE:
>>> -            raise
>>> -        return False
>>> +    families = [getattr(socket, i, None)
>>> +                for i in ('AF_INET', 'AF_INET6')
>>> +                if getattr(socket, i, None) is not None]
>>> +    for family in families:
>>> +        try:
>>> +            s = socket.socket(family, socket.SOCK_STREAM)
>>> +            s.bind(('localhost', port))
>>> +            s.close()
>>> +            return True
>>> +        except socket.error as exc:
>>> +            if exc.errno not in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
>>> +                raise
>>> +    return False
>>>
>>> closefds = os.name == 'posix'
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Feb. 16, 2017, 4:27 a.m.
> On Feb 15, 2017, at 11:24 PM, Martin von Zweigbergk <martinvonz@google.com> wrote:
> 
> On Wed, Feb 15, 2017 at 8:23 PM, Martin von Zweigbergk
> <martinvonz@google.com> wrote:
>> On Wed, Feb 15, 2017 at 8:12 PM, Augie Fackler <raf@durin42.com> wrote:
>>> 
>>>> On Feb 9, 2017, at 8:59 AM, Jun Wu <quark@fb.com> wrote:
>>>> 
>>>> # HG changeset patch
>>>> # User Jun Wu <quark@fb.com>
>>>> # Date 1486648674 28800
>>>> #      Thu Feb 09 05:57:54 2017 -0800
>>>> # Node ID 93e23f7b87a4ab456053b6ba573615be16c6c4b0
>>>> # Parent  a68510b69f413545722c086eaeb840dd5e8305b4
>>>> # Available At https://bitbucket.org/quark-zju/hg-draft
>>>> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r 93e23f7b87a4
>>>> runtests: check ports on IPv6 address
>>> 
>>> This has made test-bundle2-remote-changegroup.t flaky when run in parallel with other tests. I was able to bisect with this:
>>> 
>>> cd tests && python run-tests.py -j 100 --runs-per-test 400 test-bundle2-remote-changegroup.t
>>> 
>>> fails in about 70 out of the 400 runs. I also see it periodically when running the tests with -j120 on gcc112 from the gcc compile farm, which is a POWER8 machine with 160 hardware threads. I suspect you can reproduce it with a lower -j if you run the test enough times, but I’m up later than I should be already, so I didn’t bother logging in to a smaller linux machine to confirm.
>>> 
>>> Can you take a look?
>> 
>> Heh, I reported the flakiness on #mercurial earlier today. That's why
>> Jun sent a series of patches maybe 2 hours ago that you then queued.
>> Have you tested with those patches applied? I have not had time to
>> look at his patches, but maybe he should have explained their
>> motivation better if this is indeed the same problem that I reported.
> 
> Or maybe I just incorrectly assumed that the problems I reported with
> ports not being available had the same root cause as the flaky
> test-bundle2-remote-changegroup.t?

test-bundle2-remote-changegroup.t is still flaky for me at 1ee68, which I believe includes the series you mentioned :/


> 
>> 
>>> 
>>> Thanks!
>>> Augie
>>> 
>>>> Previously, checkportisavailable only checks ports on the IPv4 address. This
>>>> patch makes it check IPv6 as well. It'll be useful if "localhost" does not
>>>> have an IPv4 address, or its IPv4 address does not exist somehow.
>>>> 
>>>> diff --git a/tests/run-tests.py b/tests/run-tests.py
>>>> --- a/tests/run-tests.py
>>>> +++ b/tests/run-tests.py
>>>> @@ -115,13 +115,17 @@ wifexited = getattr(os, "WIFEXITED", lam
>>>> def checkportisavailable(port):
>>>>    """return true if a port seems free to bind on localhost"""
>>>> -    try:
>>>> -        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
>>>> -        s.bind(('localhost', port))
>>>> -        s.close()
>>>> -        return True
>>>> -    except socket.error as exc:
>>>> -        if not exc.errno == errno.EADDRINUSE:
>>>> -            raise
>>>> -        return False
>>>> +    families = [getattr(socket, i, None)
>>>> +                for i in ('AF_INET', 'AF_INET6')
>>>> +                if getattr(socket, i, None) is not None]
>>>> +    for family in families:
>>>> +        try:
>>>> +            s = socket.socket(family, socket.SOCK_STREAM)
>>>> +            s.bind(('localhost', port))
>>>> +            s.close()
>>>> +            return True
>>>> +        except socket.error as exc:
>>>> +            if exc.errno not in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
>>>> +                raise
>>>> +    return False
>>>> 
>>>> closefds = os.name == 'posix'
>>>> _______________________________________________
>>>> Mercurial-devel mailing list
>>>> Mercurial-devel@mercurial-scm.org
>>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>> 
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Jun Wu - Feb. 16, 2017, 4:30 a.m.
Excerpts from Augie Fackler's message of 2017-02-15 23:27:23 -0500:
> >> Heh, I reported the flakiness on #mercurial earlier today. That's why
> >> Jun sent a series of patches maybe 2 hours ago that you then queued.
> >> Have you tested with those patches applied? I have not had time to
> >> look at his patches, but maybe he should have explained their
> >> motivation better if this is indeed the same problem that I reported.
> > 
> > Or maybe I just incorrectly assumed that the problems I reported with
> > ports not being available had the same root cause as the flaky
> > test-bundle2-remote-changegroup.t?
> 
> test-bundle2-remote-changegroup.t is still flaky for me at 1ee68, which I
> believe includes the series you mentioned :/

I will investigate on gcc112. Sorry for the breakage.

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -115,13 +115,17 @@  wifexited = getattr(os, "WIFEXITED", lam
 def checkportisavailable(port):
     """return true if a port seems free to bind on localhost"""
-    try:
-        s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-        s.bind(('localhost', port))
-        s.close()
-        return True
-    except socket.error as exc:
-        if not exc.errno == errno.EADDRINUSE:
-            raise
-        return False
+    families = [getattr(socket, i, None)
+                for i in ('AF_INET', 'AF_INET6')
+                if getattr(socket, i, None) is not None]
+    for family in families:
+        try:
+            s = socket.socket(family, socket.SOCK_STREAM)
+            s.bind(('localhost', port))
+            s.close()
+            return True
+        except socket.error as exc:
+            if exc.errno not in (errno.EADDRINUSE, errno.EADDRNOTAVAIL):
+                raise
+    return False
 
 closefds = os.name == 'posix'