icinga2
icinga2 copied to clipboard
Icinga
WIP: Change the order of the API runtime object sync on reconnect
Edit (@Al2Klimov)
Rationale: see https://github.com/Icinga/icinga2/pull/7786#issuecomment-1401788118
closes #10000
@htriem Please either state the purpose of the PR or close it if you don't consider it important (anymore).
What's the purpose of the change? Does it fix a bug? Is it a performance improvement?
If there's a bug, it fixes it at the cost of performance. It re-orders runtime updates for pretty much the same reasons as we order objects while loading config initially. Once Henrik rebased this and there's something to test I maybe find a way to provoke the bug if any.
I don't really understand. There must be a reason why this PR was created and you must have had a reason why you consider it a good concept.
Damn, the reason I had in mind has been removed by 28c29c1fbc6cd6239390eaf9663b0242c000af9c. For now feel free to consider it a just-to-be-sure.
For now feel free to consider it a just-to-be-sure.
So the purpose is to increase the robustness of the object config sync?
I've looked at the change, and one situation where I can imagine this might help would be that while a child node is offline, a host and service are created for example. If the child node reconnect, could it happen without that change that the service is sent first and then fails to load? But haven't tried any of this, just wild guessing from looking at the code.
But really, if there's a proposed change, there should be a description with a motivation for the change. It shouldn't need someone else looking at the code and guessing the purpose.
That was also my guess, but I wanted to first test it once there is something to test. Apropos...
Wouldn't you test if something is broken on master or a release? So that can be done before anyone puts effort into updating an old PR.
Damn, the reason I had in mind has been removed by 28c29c1
Hence:
@htriem Please either state the purpose of the PR
@julianbrost OK, now I have enough information to give you an answer:
#9752 shouldn't happen, right? But it happens – a comment's host is missing. I thought, maybe the host is only missing yet. Maybe it should just get send over the wire before the comment itself, hence this PR. But https://github.com/Icinga/icinga2/pull/9873#issuecomment-1753228926 says no.