Skip to content

CA-410001: Check rrdi.rrd to avoid ds duplicate #6449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2025

Conversation

BengangY
Copy link
Contributor

@BengangY BengangY commented Apr 30, 2025

CA-391651 replaced the function rrd_add_ds with an unsafe function
rrd_add_ds_unsafe in rrdd_monitor.ml. Although it has checked if the new
ds exists in rrdi.dss, if a ds exists in rrdi.rrd but not in rrdi.dss, it
leads the ds duplicates twice in rrdi.rrd (E.g. when rrdd plugin starts,
it loads local rrdd backup file into rrdi.rrd but leaves rrdi.dss empty).

Solution:
Filter out new_enabled_dss based on rrdi.rrd instead of rrdi.dss.

@robhoes
Copy link
Member

robhoes commented Apr 30, 2025

@last-genius, what do you think about this?

@BengangY BengangY force-pushed the private/bengangy/CA-410001 branch from 4c928e8 to d7c080a Compare May 6, 2025 07:44
@BengangY BengangY marked this pull request as ready for review May 6, 2025 07:58
@BengangY BengangY force-pushed the private/bengangy/CA-410001 branch 2 times, most recently from 9c464bf to 08818b1 Compare May 6, 2025 10:11
@robhoes
Copy link
Member

robhoes commented May 6, 2025

Does the commit message still match the implementation?

@BengangY BengangY force-pushed the private/bengangy/CA-410001 branch from 08818b1 to a81be47 Compare May 7, 2025 07:40
CA-391651 replaced the function `rrd_add_ds` with an unsafe function
`rrd_add_ds_unsafe` in `rrdd_monitor.ml`. Although it has checked if the new
ds exists in `rrdi.dss`, if a ds exists in `rrdi.rrd` but not in `rrdi.dss`, it
leads the ds duplicates twice in `rrdi.rrd` (E.g. when rrdd plugin starts,
it loads local rrdd backup file into `rrdi.rrd` but leaves `rrdi.dss` empty).

Solution:
Filter out `new_enabled_dss` based on `rrdi.rrd` instead of `rrdi.dss`.

Signed-off-by: Bengang Yuan <bengang.yuan@cloud.com>
@BengangY BengangY force-pushed the private/bengangy/CA-410001 branch from a81be47 to 6f9a8db Compare May 7, 2025 08:05
@BengangY BengangY changed the title CA-410001: Use rrd_add_ds to avoid ds duplicate CA-410001: Check rrdi.rrd to avoid ds duplicate May 7, 2025
@BengangY
Copy link
Contributor Author

BengangY commented May 7, 2025

Does the commit message still match the implementation?

Just updated.

@BengangY BengangY added this pull request to the merge queue May 7, 2025
Merged via the queue into xapi-project:master with commit fed5c07 May 7, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants