Skip to content

Test/Encoding: Simplify and Parallelize Corpus Checking#52646

Closed
NitzanMordhai wants to merge 1 commit intoceph:mainfrom
NitzanMordhai:wip-nitzan-corpus-tools-update
Closed

Test/Encoding: Simplify and Parallelize Corpus Checking#52646
NitzanMordhai wants to merge 1 commit intoceph:mainfrom
NitzanMordhai:wip-nitzan-corpus-tools-update

Conversation

@NitzanMordhai
Copy link
Contributor

@NitzanMordhai NitzanMordhai commented Jul 26, 2023

By leveraging parallel processing capabilities during corpus checks, we can speed up the test execution.
Currently, the tests are running slowly, posing challenges in identifying mistakes within the encode-decode area and visualizing errors. Implementing the following changes will result in a faster and more efficient process, making it easier to detect and address potential issues.

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@NitzanMordhai NitzanMordhai force-pushed the wip-nitzan-corpus-tools-update branch 3 times, most recently from 03f3cb3 to 36c9c7e Compare July 26, 2023 08:00
By leveraging parallel processing capabilities during corpus checks, we can
speed up the test execution.
Currently, the tests are running slowly, posing challenges in identifying
mistakes within the encode-decode area and visualizing errors.
Implementing the following changes will result in a faster and more efficient
process, making it easier to detect and address potential issues.

Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>
@NitzanMordhai NitzanMordhai force-pushed the wip-nitzan-corpus-tools-update branch from 36c9c7e to 54532aa Compare July 27, 2023 09:07
@NitzanMordhai
Copy link
Contributor Author

jenkins test make check

@github-actions
Copy link

github-actions bot commented Dec 4, 2023

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Dec 4, 2023
@github-actions
Copy link

github-actions bot commented Jan 3, 2024

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Jan 3, 2024
@rzarzynski rzarzynski reopened this Aug 15, 2024
@github-actions github-actions bot added script and removed stale labels Aug 15, 2024
Copy link
Contributor

@rzarzynski rzarzynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM. The comments are nits.

Taking the complexity into account, I wonder whether we ensure the entire machinery continues to work, similarly to what we do for valgrind testing – there is a test intentionally causing a leak just to verify it's being caught.

shift

# import the corpus
../src/test/encoding/import.sh \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need import.sh after these changes. I see only readable.sh gets removed.

git commit --signoff --message=${version}
git remote add cc git@github.com:ceph/ceph-object-corpus.git
# Check if the remote already exists
if ! git remote | grep -q "cc"; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

def process_type(file_path, type):
print(f"dencoder test for {file_path}")
cmd1 = [CEPH_DENCODER, "type", type, "import", file_path, "decode", "dump_json"]
cmd2 = [CEPH_DENCODER, "type", type, "import", file_path, "decode", "encode", "decode", "dump_json"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these 2 lines are the crux of the entire idea, and thus deserve more intention-conveying names (or a comment). Something like cmd_decode_recorded_bytestream and cmd_decode_reencoded_bytestream? Still, they are not so good.

@rzarzynski rzarzynski requested a review from ljflores August 15, 2024 11:38
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Oct 14, 2024
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants