Checkable#ProcessCheckResult(): only clean up ack comments older than check result

[Al2Klimov]

Normally if for some reason an ack comment still exists on a checkable not
acked anymore, still clean it up. But while replaying log config objects
incl. ack comments come before check results and acks. I.e. 1) ack comment,
2) DOWN check result and 3) ack. Not 1) DOWN check result, 2) ack and 3) ack
comment. So the checkable is temporarily not acked, but already has the ack
comment. In this case the DOWN check result which is older than the ack
comment shall not clean up the latter.

fixes #9652
closes #9709

TODO

• testing (@Al2Klimov)
• testing (customer)

[julianbrost]

Haven't done any testing myself so far but the change looks fine logic-wise.

[Wintermute2k6]

From the latest Testbuild it seems to have improved something:
On a host there was a service with incoherent history entries / screenshots where provided internally
One Host all Ack vanished
Another Host where Ack vanished (shown in Logs , but in History missing, during controll look, was the Host automatically removed from the Director at 13 o'clock)
3 other hosts were synced succesfully

[Al2Klimov]

Nice!

@julianbrost 2.14? For the customer's sake?

[julianbrost]

We should do this as this fixes a problem we could reproduce independently. Remaining question: does it fix #9652? "seems to have improved something" doesn't sound very conclusive to me.

[Al2Klimov]

IMAO it fixes that:

Yes, the customer has the IDO (which we won’t fix unless objectively necessarily) which gets written the wrong timestamps:

But it's "just" a display problem. I mean, you could argue that the UNACK actually happened at that late time on master2.

Even if I'm wrong: a 90% fix is better than no fix at all.

Also:

They just shouldn’t shutdown the master for so long.

(c) Eric

[julianbrost]

Are all the remaining problems mentioned in #9718 (comment) IDO problems?

[Al2Klimov]

I think so.