osd, tools, kv: non-aggressive, on-line trimming of accumulated dups#47046
osd, tools, kv: non-aggressive, on-line trimming of accumulated dups#47046
Conversation
| } | ||
|
|
||
| while (!dups.empty()) { | ||
| // we can hit an inflated `dups` b/c of https://tracker.ceph.com/issues/53729 |
There was a problem hiding this comment.
this approach looks good to me!
should update the osd_pg_log_dups_tracked docs since the meaning of the option is a bit different now - we'll have either that many log entries, or if max log entries is lower, that many dup entries in addition to log entries.
12816a8 to
0b81d15
Compare
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
0b81d15 to
511bb49
Compare
|
jenkins test make check |
1 similar comment
|
jenkins test make check |
|
@rzarzynski the change does not compile. |
|
|
jenkins test make check |
To be able to recreate and test pg log duplicate entries, a new option
added to the COT: --op pg-log-inject-dups we will also need to provide
--file json_arry of dups, it can get as many dups that need to be inject
the json for dups is in the following format:
{"reqid": "client.n.n:n", "version": "n'n", "user_version": n, "return_code": n}
Signed-off-by: Nitzan Mordechai <nmordec@redhat.com>
Fixes: https://tracker.ceph.com/issues/53729 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Will be needed for dealing with the conf. Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Fixes: https://tracker.ceph.com/issues/53729 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
67d2c87 to
0a986bf
Compare
|
jenkins test api |
1 similar comment
|
jenkins test api |
|
jenkins test api |
1 similar comment
|
jenkins test api |
|
Tracking the API test failures here: https://tracker.ceph.com/issues/57225 @epuertat FYI |
|
jenkins test api |
… dups This commit aggregates changes for multiple PR: * Offline: ceph#46630 * Online: ceph#47046 * Offline fix: ceph#46706 * Online fix: ceph#47688 * Offline fix: ceph#46631 * Online fix: ceph#47701 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
… dups This commit aggregates changes for multiple PR: * Offline: ceph#46630 * Online: ceph#47046 * Offline fix: ceph#46706 * Online fix: ceph#47688 * Offline fix: ceph#46631 * Online fix: ceph#47701 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
… dups This commit aggregates changes for multiple PR: main ---- * Offline: ceph#46630 * Online: ceph#47046 quincy ------ * Offline fix: ceph#46706 * Online fix: ceph#47688 pacific ------- * Offline fix: ceph#46631 * Online fix: ceph#47701 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
… dups This commit aggregates changes for multiple PR: main ---- * Offline: ceph#46630 * Online: ceph#47046 quincy ------ * Offline fix: ceph#46706 * Online fix: ceph#47688 pacific ------- * Offline fix: ceph#46631 * Online fix: ceph#47701 Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
|
@rzarzynski can you take a look at this failure? It may be related: Looks like it first occurred in #46561, but then Nitzan corrected it. All other occurrences of this failure have happened when this PR was tested. @NitzanMordhai does this sound right to you? |
|
@ljflores: thanks for finding this!
|
@rzarzynski the new for loop in that PR is a bit different from the original one that I have done while I added that test, it also checks the osd-pg-log-trim-max, I need to fix the test accordingly to that trim dups. |
Change the number of dups trimmied according to the new loop. Signed-off-by: Nitzan Mordechai <nmordech@redhat.com>
|
|
Rados suite review: Failures, unrelated: Details: |
|
@rzarzynski @ljflores , regarding backporting this one to downstream octopus please may I get an opinion? To backport commit 0a986bf we need to pull in stuff from the RocksDBStore.h file Assuming the header file changes are pulled in to get it to compile, is there anything that comes to mind that may be a problem regarding the backport to Octopus? From what I see, the rest of the commits in https://github.com/ceph/ceph/pull/47046/commits can be backported to Octopus without much issues. |
|
@nkshirsagar: there is an alternative. The commit you mentioned (0a986bf) is solely about adding new debugs. If any of them brings problematic backport dependencies, we could skip it. |
|
@rzarzynski thanks, I've got this backport to build on top of octopus 15.2.17. (quick note about this in src/tools/ceph_objectstore_tool.cc, CephContext * being a boost intrusive pointer, I needed cct.get() like in the patch, but the ObjectStore *fs is used as the raw pointer it is in octopus (its a smart pointer in the patch that went to main but a raw pointer in Octopus, so here I'm using just fs and not fs.get()) Not many other changes apart from manual resolution of the other patches that did not cleanly cherry-pick from https://github.com/ceph/ceph/pull/47046/commits. (first 4 were clean cherry-pick in fact) Manually adding the debugs of 0a986bf was easier than cherry-picking it because then I did not need to take in the header file changes to the RocksDBStore.h file. Please let me know what you think of this final patch which I want to patch into downstream Octopus. |
|
@nkshirsagar: thanks, looks good! |
Hi @rzarzynski o/ I've been trying to test this patch in Octopus. I'm not sure if I am getting expected results. Please see this link for the detailed testing - https://pastebin.com/MNMWh01i (I used 500k dups in the json file instead of 50 million) |
I have managed to validate this patch in Octopus. Injected dups using this json and then started IO using rbd bench-write, Hence, I will now submit this patch (#47046 (comment)) into our downstream Octopus package. |
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. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows