Conversation
|
jenkins test api |
1 similar comment
|
jenkins test api |
|
jenkins test make check arm64 |
|
jenkins test api |
|
nit: please retab the code. mix of tabs and spaces makes it hard to review |
2970b51 to
509376b
Compare
batrick
left a comment
There was a problem hiding this comment.
Please title your commits according to the conventions of this project. This is required for your PR to be merged.
See this article on GitHub on how to amend commits and update your pull request.
batrick
left a comment
There was a problem hiding this comment.
Design doc should not be in google docs. Please move it to doc/dev.
Empty commit messages only including a commit title is also not acceptable.
0dd968a to
b1c8d63
Compare
|
jenkins test make check arm64 |
1 similar comment
|
jenkins test make check arm64 |
|
jenkins test make check |
|
jenkins test api |
56f7f7c to
7fbbdf1
Compare
|
jenkins test api |
|
jenkins test make check arm64 |
4e61a08 to
8fc2fc2
Compare
|
regression: https://pulpito.ceph.com/yuvalif-2025-05-19_09:57:15-rgw-wip_s3_full_object_dedup_merged-distro-default-smithi/
not sure about: |
8fc2fc2 to
560a080
Compare
|
jenkins test make check arm64 |
|
jenkins test windows |
librgw is failing due to a crash on shutdown. can reproduce locally: |
@yuvalif seeing that in rgw/notifications jobs for other runs too, so opened https://tracker.ceph.com/issues/71390. seems something in the notification path is preventing clean shutdown. could you take a look? |
5649610 to
d10bf97
Compare
|
Please correct the spelling in the PR title. |
|
teuthology: https://pulpito.ceph.com/yuvalif-2025-05-21_09:48:40-rgw-wip_s3_full_object_dedup_shutdown-distro-default-smithi/
|
8802cca to
76fabe6
Compare
|
jenkins test make check |
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
Design Document: https://docs.google.com/document/d/152VyCTR2NlZ6ongbe6-CJfP4qxr1_zH83FB_WukWD7c Signed-off-by: Gabriel BenHanokh <gbenhano@redhat.com>
76fabe6 to
081bda7
Compare
Signed-off-by: Yuval Lifshitz <ylifshit@ibm.com>
58a5c47 to
895121f
Compare
|
| #include <string> | ||
|
|
||
| namespace rgw::dedup { | ||
| static constexpr const char* DEDUP_POOL_NAME = "rgw_dedup_pool"; |
There was a problem hiding this comment.
this pool name needs to be include the zone id. you can put multiple zones on the same cluster, and their pools should not conflict
There was a problem hiding this comment.
@benhanokh append somethin like:
static_cast<rgw::sal::RadosStore*>(driver)->svc()->zone->zone_id();
to the pool name.
- need to make sure that the zone id has the zone-group and realm in it, o/w it may not be unique in the cluster.
- note that the zone id may change after a reload, so the reload should probably terminate the dedup process if we reload. also need to do that cleanup in this case
| d_runner = std::thread(&Background::run, this); | ||
| const auto rc = ceph_pthread_setname("dedup_bg"); |
There was a problem hiding this comment.
this is calling ceph_pthread_setname() from the main thread, not the background thread
the consequence is that it changes the process name from radosgw to dedup_bg, meaning that killall radosgw no longer works
edit: tracking in https://tracker.ceph.com/issues/71543
There was a problem hiding this comment.
@benhanokh can you please fix it in a small dedicated PR?
(this is causing lots of inconviences in the dev process)
Design Document:
https://docs.google.com/document/d/152VyCTR2NlZ6ongbe6-CJfP4qxr1_zH83FB_WukWD7c
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an
xbetween the brackets:[x]. Spaces and capitalization matter when checking off items this way.Checklist
Show available Jenkins commands
jenkins test classic perfJenkins Job | Jenkins Job Definitionjenkins test crimson perfJenkins Job | Jenkins Job Definitionjenkins test signedJenkins Job | Jenkins Job Definitionjenkins test make checkJenkins Job | Jenkins Job Definitionjenkins test make check arm64Jenkins Job | Jenkins Job Definitionjenkins test submodulesJenkins Job | Jenkins Job Definitionjenkins test dashboardJenkins Job | Jenkins Job Definitionjenkins test dashboard cephadmJenkins Job | Jenkins Job Definitionjenkins test apiJenkins Job | Jenkins Job Definitionjenkins test docsReadTheDocs | Github Workflow Definitionjenkins test ceph-volume allJenkins Jobs | Jenkins Jobs Definitionjenkins test windowsJenkins Job | Jenkins Job Definitionjenkins test rook e2eJenkins Job | Jenkins Job Definition