Skip to content
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

Fix broken downtime comment sync #10000

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yhabteab
Copy link
Member

@yhabteab yhabteab commented Feb 12, 2024

Sync all objects in activation priority descending order, otherwise downtimes,
comments etc. might be synced before their respective checkables, which
would result in comments/downtimes being ignored by the other endpoint
since it doesn't yet know about their checkables. Since the runtime
config updates event doesn't trigger a reload on the remote endpoint,
theses objects won't be synced again til the next reload/reconnect.

~/master2/icinga2 (bundled-cluster-fixes ✗) ls prefix/var/lib/icinga2/api/packages/_api/*/conf.d/downtimes | wc -l
    3501
~/master2/icinga2 (bundled-cluster-fixes ✗) curl -sSku root:icinga 'https://localhost:5666/v1/objects/downtimes?pretty=1' | grep ' "attrs": {' | wc -l
    1501
~/master1/icinga2 (bundled-cluster-fixes ✗) curl -sSku root:icinga 'https://localhost:5665/v1/objects/downtimes?pretty=1' | grep ' "attrs": {' | wc -l
    3501

After master2 reload:

~/master2/icinga2 (bundled-cluster-fixes ✗) curl -sSku root:icinga 'https://localhost:5666/v1/objects/downtimes?pretty=1' | grep ' "attrs": {' | wc -l
    3501

closes #7786
closes #9873

Sync all objects in priority descending order, otherwise downtimes,
comments etc. might be synced before their respective checkables, which
would result in comments/downtimes being ignored by the other endpoint
since it doesn't yet know about their checkables. Since the runtime
config updates event doesn't trigger a reload on the remote endpoint,
theses objects won't be synced again til the next reload.
@yhabteab yhabteab added bug Something isn't working area/distributed Distributed monitoring (master, satellites, clients) ref/IP area/runtime Downtimes, comments, dependencies, events labels Feb 12, 2024
@cla-bot cla-bot bot added the cla/signed label Feb 12, 2024
// trigger a reload on the remote endpoint, these objects won't be synced again til the next reload.
std::vector<Type::Ptr> types = Type::GetAllTypes();
std::sort(types.begin(), types.end(), [](const Type::Ptr& a, const Type::Ptr& b) {
return a->GetActivationPriority() > b->GetActivationPriority();
Copy link
Member

Choose a reason for hiding this comment

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

Better call Type#GetLoadDependencies().

Copy link
Member Author

Choose a reason for hiding this comment

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

What is Type#GetLoadDependencies() supposed to bring me here? It would be nice if you could explain in more detail why you think activation priority is not right instead of simply suggesting "better call Type#GetLoadDependencies()".

Copy link
Member

Choose a reason for hiding this comment

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

Activation priority is just about in which order objects start doing things. In contrast, Type#GetLoadDependencies() reflects the load_after T; we discussed offline. It fetches the types which must be loaded before the given type.

Copy link
Member

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/distributed Distributed monitoring (master, satellites, clients) area/runtime Downtimes, comments, dependencies, events bug Something isn't working cla/signed ref/IP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants