Page MenuHomeSoftware Heritage

tests: only flush() the kafka journal writer once per test
ClosedPublic

Authored by douardda on Oct 18 2022, 8:07 PM.

Details

Summary

instead of flushing it n times per test. Since the call to kafka
Producer.flush() takes about 1s, reducing the number of calls to this
method significantly reduce the execution time of the tests.

This required a small refactoring of the JournalBackfiller class to make
the journal writer live out of the scope of the run() so the test can
access the journal writer instance and call the flush() method.

Requires swh-journal >= 1.2.0 (see D8705).

Diff Detail

Repository
rDSTO Storage manager
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 D8740 (id=31500)

Rebasing onto c1c2dbf01b...

Current branch diff-target is up to date.
Changes applied before test
commit fa4e0dcac61ebd31fadd8b93e68f70677a8e7241
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Oct 18 15:43:17 2022 +0200

    tests: only flush() the kafka journal writer once per test
    
    instead of flushing it n times per test. Since the call to kafka
    Producer.flush() takes about 1s, reducing the number of calls to this
    method significantly reduce the execution time of the tests.
    
    This required a small refactoring of the JournalBackfiller class to make
    the journal writer live out of the scope of the run() so the test can
    access the journal writer instance and call the flush() method.
    
    Requires swh-journal >= 1.2.0

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

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 18 2022, 8:16 PM
Harbormaster failed remote builds in B32446: Diff 31500!

Build has FAILED

Patch application report for D8740 (id=31500)

Rebasing onto c1c2dbf01b...

Current branch diff-target is up to date.
Changes applied before test
commit fa4e0dcac61ebd31fadd8b93e68f70677a8e7241
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Oct 18 15:43:17 2022 +0200

    tests: only flush() the kafka journal writer once per test
    
    instead of flushing it n times per test. Since the call to kafka
    Producer.flush() takes about 1s, reducing the number of calls to this
    method significantly reduce the execution time of the tests.
    
    This required a small refactoring of the JournalBackfiller class to make
    the journal writer live out of the scope of the run() so the test can
    access the journal writer instance and call the flush() method.
    
    Requires swh-journal >= 1.2.0

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

rebase and update requirements for journal>=1.2

Build has FAILED

Patch application report for D8740 (id=31543)

Could not rebase; Attempt merge onto 784f730e3a...

Updating 784f730e..37ffc642
Fast-forward
 requirements-swh-journal.txt           |  2 +-
 swh/storage/backfill.py                | 18 ++++++++-----
 swh/storage/interface.py               | 17 +++++++++++-
 swh/storage/replay.py                  | 41 +++++++++++++++++++++++-----
 swh/storage/tests/test_backfill.py     |  5 ++++
 swh/storage/tests/test_kafka_writer.py |  6 +++++
 swh/storage/tests/test_replay.py       | 49 +++++++++++++++++++++++++++++++---
 7 files changed, 119 insertions(+), 19 deletions(-)
Changes applied before test
commit 37ffc642a810063a91dc31692273964d4519ab12
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Oct 18 15:43:17 2022 +0200

    tests: only flush() the kafka journal writer once per test
    
    instead of flushing it n times per test. Since the call to kafka
    Producer.flush() takes about 1s, reducing the number of calls to this
    method significantly reduce the execution time of the tests.
    
    This required a small refactoring of the JournalBackfiller class to make
    the journal writer live out of the scope of the run() so the test can
    access the journal writer instance and call the flush() method.
    
    Requires swh-journal >= 1.2.0

commit fe0eaee8718bfd33d8ba383242e3abb09ad59634
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Oct 21 14:09:07 2022 +0200

    Make the replayer not crash on kafka messages that fail to be converted as model objects
    
    for example, there are a few kafka directory messages in the current
    production kafka cluster which entries contain the same name twice,
    preventing the Directory model object from being created at all,
    which makes the replayer crash.
    
    This change makes the replayer able to handle such cases. When the model
    object creation fails with a ValueError, the error is reported in the
    (redis) error reporter, but the replaying process continue.
    
    Since there is no model object, the error is reported with a crafted
    error key of the form "{object_type}:{object_id}" if an object id is
    present in the data structure, or "{object_type}:uuid:{uuid4}" if such
    an id is not even present. For the record, the standard error key in
    redis for a model object is it's swhid (if any).

commit 242e37a7be35b931986724c5adc75dc90284f0fd
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Oct 21 14:05:23 2022 +0200

    Add a comment that should have been "kept" from 850a7553b

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

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 21 2022, 4:47 PM
Harbormaster failed remote builds in B32497: Diff 31543!

Build has FAILED

Patch application report for D8740 (id=31544)

Could not rebase; Attempt merge onto 784f730e3a...

Updating 784f730e..e87b81d7
Fast-forward
 requirements-swh-journal.txt           |  2 +-
 swh/storage/backfill.py                | 17 +++++++-----
 swh/storage/interface.py               | 17 +++++++++++-
 swh/storage/replay.py                  | 41 +++++++++++++++++++++++-----
 swh/storage/tests/test_backfill.py     |  5 ++++
 swh/storage/tests/test_kafka_writer.py |  6 +++++
 swh/storage/tests/test_replay.py       | 49 +++++++++++++++++++++++++++++++---
 7 files changed, 119 insertions(+), 18 deletions(-)
Changes applied before test
commit e87b81d7720ad2cf8dcf02db7ccdc8da9e8213ec
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Oct 18 15:43:17 2022 +0200

    tests: only flush() the kafka journal writer once per test
    
    instead of flushing it n times per test. Since the call to kafka
    Producer.flush() takes about 1s, reducing the number of calls to this
    method significantly reduce the execution time of the tests.
    
    This required a small refactoring of the JournalBackfiller class to make
    the journal writer live out of the scope of the run() so the test can
    access the journal writer instance and call the flush() method.
    
    Requires swh-journal >= 1.2.0

commit fe0eaee8718bfd33d8ba383242e3abb09ad59634
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Oct 21 14:09:07 2022 +0200

    Make the replayer not crash on kafka messages that fail to be converted as model objects
    
    for example, there are a few kafka directory messages in the current
    production kafka cluster which entries contain the same name twice,
    preventing the Directory model object from being created at all,
    which makes the replayer crash.
    
    This change makes the replayer able to handle such cases. When the model
    object creation fails with a ValueError, the error is reported in the
    (redis) error reporter, but the replaying process continue.
    
    Since there is no model object, the error is reported with a crafted
    error key of the form "{object_type}:{object_id}" if an object id is
    present in the data structure, or "{object_type}:uuid:{uuid4}" if such
    an id is not even present. For the record, the standard error key in
    redis for a model object is it's swhid (if any).

commit 242e37a7be35b931986724c5adc75dc90284f0fd
Author: David Douard <david.douard@sdfa3.org>
Date:   Fri Oct 21 14:05:23 2022 +0200

    Add a comment that should have been "kept" from 850a7553b

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

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 21 2022, 5:04 PM
Harbormaster failed remote builds in B32498: Diff 31544!

Build is green

Patch application report for D8740 (id=31547)

Rebasing onto fe0eaee871...

Current branch diff-target is up to date.
Changes applied before test
commit 0e8da810ac962b79dda18d9cfe9f3c7ea4f9de52
Author: David Douard <david.douard@sdfa3.org>
Date:   Tue Oct 18 15:43:17 2022 +0200

    tests: only flush() the kafka journal writer once per test
    
    instead of flushing it n times per test. Since the call to kafka
    Producer.flush() takes about 1s, reducing the number of calls to this
    method significantly reduce the execution time of the tests.
    
    This required a small refactoring of the JournalBackfiller class to make
    the journal writer live out of the scope of the run() so the test can
    access the journal writer instance and call the flush() method.
    
    Requires swh-journal >= 1.2.0

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

On the outstanding sample of one execution on jenkins: 11min 44s vs 13min 39s (previous execution, before this diff)

This revision is now accepted and ready to land.Oct 21 2022, 5:45 PM