Page MenuHomeSoftware Heritage
Differential D8939

Rework the replaying exception handling
ClosedPublic
Actions

Authored by douardda on Dec 7 2022, 4:30 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDOBJSRPL11b1b233e2b1: Add tests for replay scenarios with add errors
rDOBJSRPLb0989f7ed9d7: Improve logging in replay.py
rDOBJSRPLa3e366823d81: Rework the replaying exception handling
Summary

to make it a bit more robust and give more details on what's going on in
case of errors.

Add tests for those error situations.

Also improve logging in replay.py:

Add a few debug logging statements and improve the info-level summary
statement at the end of process_replay_objects_content(), and fix tests accordingly.

Diff Detail

Event Timeline

douardda created this revision.Dec 7 2022, 4:30 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptDec 7 2022, 4:30 PM
swh-public-ci added a comment.Dec 7 2022, 4:33 PM

Build is green

Patch application report for D8939 (id=32209)

Rebasing onto 0c0fa5c733...

Current branch diff-target is up to date.
Changes applied before test
commit 82e3a9258a56a064acf05dd6cf2a6ffab90d907d
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Dec 7 16:21:15 2022 +0100

    Add tests for replay scenarios with add errors

commit 9156f6e1d5e68e195f7c98e6666102dfa61e2244
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Dec 7 16:19:11 2022 +0100

    Improve logging in replay.py
    
    Add a few debug logging statements and improve the info-level summary
    statement at the end of `process_replay_objects_content()`.
    
    Fix tests accordingly.

commit 062d603ebb5128354137beaaec35d92614d28eb4
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Nov 21 16:05:24 2022 +0100

    Rework the replaying exception handling
    
    to make it a bit more robust and give more details on what's going on in
    case of errors.
    
    Add tests for those error situations.

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

Harbormaster completed remote builds in B33163: Diff 32209.Dec 7 2022, 4:33 PM
douardda requested review of this revision.Dec 7 2022, 4:33 PM
douardda edited the summary of this revision. (Show Details)Dec 7 2022, 4:51 PM
douardda edited the summary of this revision. (Show Details)
vlorentz added a subscriber: vlorentz.Dec 8 2022, 11:31 AM

Could you use a logger instance, and add if logger.isEnabledFor(logging.DEBUG): before logger.debug statements that use hash_to_hex?

swh/objstorage/replayer/tests/test_replay_errors.py
1

2022

douardda added a comment.Dec 9 2022, 2:48 PM
In D8939#232508, @vlorentz wrote:

Could you use a logger instance,

what do you mean by "use a logger instance"?

and add if logger.isEnabledFor(logging.DEBUG): before logger.debug statements that use hash_to_hex?

sure

douardda added a comment.Dec 9 2022, 2:52 PM
In D8939#232687, @douardda wrote:
In D8939#232508, @vlorentz wrote:

Could you use a logger instance,

what do you mean by "use a logger instance"?

and add if logger.isEnabledFor(logging.DEBUG): before logger.debug statements that use hash_to_hex?

sure

Actually, why would this be needed? I mean I don't expect has_to_hex() to be a fat cpu burner, do I?

douardda added a comment.EditedDec 9 2022, 2:58 PM
In D8939#232688, @douardda wrote:
In D8939#232687, @douardda wrote:
In D8939#232508, @vlorentz wrote:

Could you use a logger instance,

what do you mean by "use a logger instance"?

and add if logger.isEnabledFor(logging.DEBUG): before logger.debug statements that use hash_to_hex?

sure

Actually, why would this be needed? I mean I don't expect has_to_hex() to be a fat cpu burner, do I?

And hash_to_hex() is already lru_cached, so I don't see the true benefit of adding these if statements everywhere...

douardda updated this revision to Diff 32252.Dec 9 2022, 3:02 PM

Bump copyright dates

swh-public-ci added a comment.Dec 9 2022, 3:05 PM

Build is green

Patch application report for D8939 (id=32252)

Rebasing onto 0c0fa5c733...

Current branch diff-target is up to date.
Changes applied before test
commit 11b1b233e2b113e622f256928ec7e6273f8e60d1
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Dec 7 16:21:15 2022 +0100

    Add tests for replay scenarios with add errors

commit b0989f7ed9d7771e81d8a8fd989d50b8fa4ce3c9
Author: David Douard <david.douard@sdfa3.org>
Date:   Wed Dec 7 16:19:11 2022 +0100

    Improve logging in replay.py
    
    Add a few debug logging statements and improve the info-level summary
    statement at the end of `process_replay_objects_content()`.
    
    Fix tests accordingly.

commit a3e366823d810220dbfac2b24cf02fc98866ff2e
Author: David Douard <david.douard@sdfa3.org>
Date:   Mon Nov 21 16:05:24 2022 +0100

    Rework the replaying exception handling
    
    to make it a bit more robust and give more details on what's going on in
    case of errors.
    
    Add tests for those error situations.

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

Harbormaster completed remote builds in B33207: Diff 32252.Dec 9 2022, 3:05 PM
vlorentz accepted this revision.Dec 15 2022, 10:04 AM
In D8939#232687, @douardda wrote:
In D8939#232508, @vlorentz wrote:

Could you use a logger instance,

what do you mean by "use a logger instance"?

I meant using the result of logging.getLogger() instead of logging directly, but you are already doing that. I don't see why I wrote that comment...

In D8939#232692, @douardda wrote:

And hash_to_hex() is already lru_cached, so I don't see the true benefit of adding these if statements everywhere...

would it be called if not for logging, though?

In D8939#232688, @douardda wrote:

Actually, why would this be needed? I mean I don't expect has_to_hex() to be a fat cpu burner, do I?

Heh you're right, it's only ~0.3µs on cache miss

This revision is now accepted and ready to land.Dec 15 2022, 10:04 AM
Closed by commit rDOBJSRPLa3e366823d81: Rework the replaying exception handling (authored by douardda). · Explain WhyDec 15 2022, 4:21 PM
This revision was automatically updated to reflect the committed changes.
douardda added a commit: rDOBJSRPLa3e366823d81: Rework the replaying exception handling.
douardda added a commit: rDOBJSRPLb0989f7ed9d7: Improve logging in replay.py.
douardda added a commit: rDOBJSRPL11b1b233e2b1: Add tests for replay scenarios with add errors.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
2 lines--
swh/
objstorage/
replayer/
105 lines93%
Loading...
tests/
31 lines99%
Loading...
301 lines100%
Loading...

Diff 32252

View Options

requirements.txt

Loading...
View Options

swh/objstorage/replayer/replay.py

Loading...
View Options

swh/objstorage/replayer/tests/test_cli.py

Loading...
View Options

swh/objstorage/replayer/tests/test_replay_errors.py

Loading...
About · Developer information · Legal