Page MenuHomeSoftware Heritage

Regularly check for EOF in Client.process() while waiting for messages
ClosedPublic

Authored by douardda on Apr 8 2022, 11:47 AM.

Details

Summary

it may happen that an EOF condition is not properly detected during
execution of the handle_messages() method, thus making the waiting step
in process() hang forever.

This adds a check for the EOF condition once every 10 timeouts while
consuming messages.

Diff Detail

Repository
rDJNL Journal infrastructure
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build has FAILED

Patch application report for D7531 (id=27317)

Rebasing onto 3771edbab3...

Current branch diff-target is up to date.
Changes applied before test
commit 784a99dc1e0e5b8440c3b52d2a832998285da70c
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 7 15:46:04 2022 +0200

    Regularly check for OEF in Client.process() while waiting for messages
    
    it may happen that an EOF condition is not properly detected during
    execution of the handle_messages() method, thus making the waiting step
    in process() hang forever.
    
    This adds a check for the EOF condition once every 10 timeouts while
    consuming messages.

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/198/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/198/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 8 2022, 11:50 AM
Harbormaster failed remote builds in B28249: Diff 27317!

fix the tests by only cheking the EOF condition if we alredy consumed at least one message.

douardda retitled this revision from Regularly check for OEF in Client.process() while waiting for messages to Regularly check for EOF in Client.process() while waiting for messages.Apr 8 2022, 4:23 PM

Build has FAILED

Patch application report for D7531 (id=27341)

Rebasing onto f514445fdd...

First, rewinding head to replay your work on top of it...
Applying: Regularly check for EOF in Client.process() while waiting for messages
Using index info to reconstruct a base tree...
M	swh/journal/client.py
Falling back to patching base and 3-way merge...
Auto-merging swh/journal/client.py
Changes applied before test
commit 0efd2e3639fc2ee7158b9e0fdad9f03513411e29
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 7 15:46:04 2022 +0200

    Regularly check for EOF in Client.process() while waiting for messages
    
    it may happen that an EOF condition is not properly detected during
    execution of the handle_messages() method, thus making the waiting step
    in process() hang forever.
    
    This adds a check for the EOF condition once every 10 timeouts while
    consuming messages.

Link to build: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/199/
See console output for more information: https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/199/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 8 2022, 4:25 PM
Harbormaster failed remote builds in B28359: Diff 27341!

Build is green

Patch application report for D7531 (id=27341)

Rebasing onto f514445fdd...

First, rewinding head to replay your work on top of it...
Applying: Regularly check for EOF in Client.process() while waiting for messages
Using index info to reconstruct a base tree...
M	swh/journal/client.py
Falling back to patching base and 3-way merge...
Auto-merging swh/journal/client.py
Changes applied before test
commit 4c6290752dc88ce59731092af43c4c21b4269041
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 7 15:46:04 2022 +0200

    Regularly check for EOF in Client.process() while waiting for messages
    
    it may happen that an EOF condition is not properly detected during
    execution of the handle_messages() method, thus making the waiting step
    in process() hang forever.
    
    This adds a check for the EOF condition once every 10 timeouts while
    consuming messages.

See https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/200/ for more details.

ardumont added inline comments.
swh/journal/client.py
299

not sure whether you need to wrap the range with a list call or not though

In [2]: list(reversed(range(10)))
Out[2]: [9, 8, 7, 6, 5, 4, 3, 2, 1, 0]

In [5]: list(range(9, -1, -1))
Out[5]: [9, 8, 7, 6, 5, 4, 3, 2, 1, 0]
307
swh/journal/client.py
299

My first version did use the `range()` function, but I find it more explicit the way I wrote (I especially do not like to have to use -1 as stop limit)

Build is green

Patch application report for D7531 (id=27479)

Rebasing onto f514445fdd...

First, rewinding head to replay your work on top of it...
Applying: Regularly check for EOF in Client.process() while waiting for messages
Using index info to reconstruct a base tree...
M	swh/journal/client.py
Falling back to patching base and 3-way merge...
Auto-merging swh/journal/client.py
Changes applied before test
commit f90e9e67a8f7d7604b3d10eac99abcd98138aa6f
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 7 15:46:04 2022 +0200

    Regularly check for EOF in Client.process() while waiting for messages
    
    it may happen that an EOF condition is not properly detected during
    execution of the handle_messages() method, thus making the waiting step
    in process() hang forever.
    
    This adds a check for the EOF condition once every 10 timeouts while
    consuming messages.

See https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/201/ for more details.

vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/journal/client.py
299

yeah, it's more readable this way

This revision is now accepted and ready to land.Apr 20 2022, 11:41 AM

Build is green

Patch application report for D7531 (id=27572)

Rebasing onto f514445fdd...

Current branch diff-target is up to date.
Changes applied before test
commit f98248dd693841e3d6896a91cc7ae5e647f175ca
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Apr 7 15:46:04 2022 +0200

    Regularly check for EOF in Client.process() while waiting for messages
    
    it may happen that an EOF condition is not properly detected during
    execution of the handle_messages() method, thus making the waiting step
    in process() hang forever.
    
    This adds a check for the EOF condition once every 10 timeouts while
    consuming messages.

See https://jenkins.softwareheritage.org/job/DJNL/job/tests-on-diff/202/ for more details.