Page MenuHomeSoftware Heritage
Differential D1918

statsd: protect access to the statsd's socket
ClosedPublic
Actions

Authored by douardda on Aug 28 2019, 10:40 AM.

Details

Reviewers
olasd
Group Reviewers
Reviewers
Commits
rDCORE220d1c470c18: statsd: protect access to the statsd's socket
Summary

to prevent erratic errors when using heavily multithreaded code.

Diff Detail

Repository
rDCORE Foundations and core functionalities
Branch
statsd
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7674
Build 11012: tox-on-jenkinsJenkins
Build 11011: arc lint + arc unit

Event Timeline

douardda created this revision.Aug 28 2019, 10:40 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptAug 28 2019, 10:40 AM
swh-public-ci added a comment.Aug 28 2019, 10:42 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCORE/job/tox/279/
See console output for more information: https://jenkins.softwareheritage.org/job/DCORE/job/tox/279/console

Harbormaster failed remote builds in B7480: Diff 6435!Aug 28 2019, 10:42 AM
vlorentz added a subscriber: vlorentz.EditedAug 28 2019, 10:45 AM

Why isn't the socket initialized by __init__?

douardda added a comment.Aug 29 2019, 1:43 PM
In D1918#44375, @vlorentz wrote:

Why isn't the socket initialized by __init__?

we could, but it adds an extra line with no obvious improvement.

douardda added a comment.Aug 29 2019, 1:45 PM
In D1918#44375, @vlorentz wrote:

Why isn't the socket initialized by __init__?

And it was written like this before this refactoring (creation of the socket on demand), so I kept it as is.

olasd requested changes to this revision.Sep 2 2019, 4:32 PM
olasd added a subscriber: olasd.

You'll need to update the tests to set _socket instead.

This revision now requires changes to proceed.Sep 2 2019, 4:32 PM
vlorentz added a comment.Sep 3 2019, 10:39 AM

with no obvious improvement.

It removes the need for a lock and a property.

douardda added a comment.Sep 5 2019, 9:38 AM
In D1918#44939, @vlorentz wrote:

with no obvious improvement.

It removes the need for a lock and a property.

Nope, one thread could call close_socket while another one is still doing stuff...

douardda updated this revision to Diff 6598.Sep 5 2019, 9:45 AM

update tests, as pointed out by olasd

swh-public-ci added a comment.Sep 5 2019, 9:47 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/281/ for more details.

Harbormaster completed remote builds in B7674: Diff 6598.Sep 5 2019, 9:47 AM
douardda updated this revision to Diff 6619.Sep 6 2019, 11:13 AM

rebase

swh-public-ci added a comment.Sep 6 2019, 11:15 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/285/ for more details.

Harbormaster completed remote builds in B7695: Diff 6619.Sep 6 2019, 11:15 AM
olasd accepted this revision.Sep 6 2019, 11:15 AM
This revision is now accepted and ready to land.Sep 6 2019, 11:15 AM
Closed by commit rDCORE220d1c470c18: statsd: protect access to the statsd's socket (authored by douardda). · Explain WhySep 6 2019, 11:18 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
swh/
core/
27 lines
tests/
28 lines

Diff 6598

View Options

swh/core/statsd.py

Loading...
View Options

swh/core/tests/test_statsd.py

Loading...
About · Developer information · Legal