this is not an ideal solution since it's using a global variable
(swh.objstorage.replayer.replay.REPORTER) to activate the replaying
error reporting feature, but according the nature of tenacity (used to
implement retry capacity of interactions with the source and destination
objstorages), there is no easy solution (that I can see).
Details
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Maniphest Tasks
- T3693: Provide a mecanism to report (with persistence) objects that fails to get replayed (mirror)
- Commits
- rDOBJSRPLf5051ce1cbf4: Add support for a redis-based reporter for failed replayed objects
Diff Detail
Diff Detail
- Repository
- rDOBJSRPL Content replayer
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build is green
Patch application report for D6693 (id=24313)
Could not rebase; Attempt merge onto 720b7af2bf...
Updating 720b7af..10f620a Fast-forward mypy.ini | 3 + requirements-test.txt | 2 + requirements.txt | 1 + swh/objstorage/replayer/cli.py | 5 + swh/objstorage/replayer/replay.py | 111 ++++++++++++-------- swh/objstorage/replayer/tests/test_cli.py | 167 ++++++++++++++++++++++++------ 6 files changed, 216 insertions(+), 73 deletions(-)
Changes applied before test
commit 10f620acc13d9490f44e282329d84d8568483f90
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 18:05:01 2021 +0100
Add support for a redis-based reporter for failed replayed objects
this is not an ideal solution since it's using a global variable
(swh.objstorage.replayer.replay.REPORTER) to activate the replaying
error reporting feature, but according the nature of tenacity (used to
implement retry capacity of interactions with the source and destination
objstorages), there is no easy solution (that I can see).
commit cbb915109c82a63d98c4006e19dbc3d2bfa572a5
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 18:02:54 2021 +0100
Add doctrings and comments in test_cli.py
commit 604e00e30439ba5bd9dce24fd505637da81dd75f
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 17:49:00 2021 +0100
Rework the retry and reporting system in replay.py
split the 'copy_object()' in 2 parts, 'get_object()' and 'put_object()'
and make each of these decorated by '@content_replay_retry' (instead of
the whole 'copy_object()'), so that a failing 'put_object' does not
trigger getting the object from the src objstorage again.
Also, only log in statsd the end result (error or success) instead of
logging each attempt. We don't need these intermediate results in
statsd, and it makes them much harder to use in a dashboard.
Last, for the sake of implementation ease, use function name as
"operation" tag in stastds reports (so "get_object", "put_object" and
"obj_in_storage") instead of "copy" and "in_dst".
Doing so, the 'operation' argument of the ReplayError exception has been
dropped.
commit 002443567791de9ffef1a2b443eb7471d120bebe
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 17:36:27 2021 +0100
Small code refactoring in test_cli
make '_fill_objstorage_and_kafka()' take the actual objstorage as
argument, instead of the dict of objstorages ("src", "dst"). The "dst"
objstorage is not used in the function, and it make the intent of this
later clearer.See https://jenkins.softwareheritage.org/job/DOBJSRPL/job/tests-on-diff/17/ for more details.
Comment Actions
there is no easy solution (that I can see).
There is one: moving all functions in replay.py to their own class, and set tenacity when instantiating the class. But I don't think it's worth it.
Comment Actions
Build is green
Patch application report for D6693 (id=24435)
Could not rebase; Attempt merge onto 720b7af2bf...
Updating 720b7af..f5051ce Fast-forward mypy.ini | 3 + requirements-test.txt | 2 + requirements.txt | 1 + swh/objstorage/replayer/cli.py | 5 + swh/objstorage/replayer/replay.py | 112 +++++++++++------- swh/objstorage/replayer/tests/test_cli.py | 167 ++++++++++++++++++++++----- swh/objstorage/replayer/tests/test_statsd.py | 119 +++++++++++++++++++ 7 files changed, 336 insertions(+), 73 deletions(-) create mode 100644 swh/objstorage/replayer/tests/test_statsd.py
Changes applied before test
commit f5051ce1cbf4f8e5a73c57a38c689ea69d8c282e
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 18:05:01 2021 +0100
Add support for a redis-based reporter for failed replayed objects
this is not an ideal solution since it's using a global variable
(swh.objstorage.replayer.replay.REPORTER) to activate the replaying
error reporting feature, but according the nature of tenacity (used to
implement retry capacity of interactions with the source and destination
objstorages), there is no easy solution (that I can see).
commit a7bd6bc4427bf35741c8953751ec1330f91445df
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 18:02:54 2021 +0100
Add doctrings and comments in test_cli.py
commit 8098798820bb2025f9bfc7c6b2b46e08a818b797
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 17:49:00 2021 +0100
Rework the retry and reporting system in replay.py
split the 'copy_object()' in 2 parts, 'get_object()' and 'put_object()'
and make each of these decorated by '@content_replay_retry' (instead of
the whole 'copy_object()'), so that a failing 'put_object' does not
trigger getting the object from the src objstorage again.
Also, only log in statsd the end result (error or success) instead of
logging each attempt. We don't need these intermediate results in
statsd, and it makes them much harder to use in a dashboard.
Last, for the sake of implementation ease, use function name as
"operation" tag in stastds reports (so "get_object", "put_object" and
"obj_in_storage") instead of "copy" and "in_dst".
Doing so, the 'operation' argument of the ReplayError exception has been
dropped.
commit 1d8ea80c7d01847dba16343508b74a037a1da864
Author: David Douard <david.douard@sdfa3.org>
Date: Wed Dec 1 17:28:56 2021 +0100
Add tests for expected statsd reports during a content replay session
commit 002443567791de9ffef1a2b443eb7471d120bebe
Author: David Douard <david.douard@sdfa3.org>
Date: Thu Nov 25 17:36:27 2021 +0100
Small code refactoring in test_cli
make '_fill_objstorage_and_kafka()' take the actual objstorage as
argument, instead of the dict of objstorages ("src", "dst"). The "dst"
objstorage is not used in the function, and it make the intent of this
later clearer.See https://jenkins.softwareheritage.org/job/DOBJSRPL/job/tests-on-diff/23/ for more details.