Stored 'lastupdate' being approximate can cause inconsistent missing data about metrictank

[Dieterbe]

I thought this exact issue has come up before, but I couldn't find an issue or PR.
Could only find some related issues (e.g. #550, #1532)

sidenote: somewhat confusingly, LastSave is the property that governs writes to the persistent index (subject to "update-interval"), and it causes the full metricdef to be written, but yeah i can see how this would be a problem for the lastUpdate field specifically.

Option 3 seems like the "obviously correct" solution to me. In fact, it essentially "extends" the exact same solution of #1532, taking the "extra allowance" used at query time and enabling it at index loading time. actually for the former fix to function correctly, it requires the latter fix (shortly after startup, anyway)
I'm not too worried about this causing a lot more index entries to be loaded, as the update-interval is typically a low value (couple of hours), much shorter to the the pruning durations used in index-rules.conf

So yes, let's do number 3. should be a very easy fix I think.

[shanson7]

actually for the former fix to function correctly, it requires the latter fix (shortly after startup, anyway)

Not sure I understand this bit. It seems to me that Option 3 would obviate the need for #1532 (and would support the large number of Find style operations that have been added since). So implementing Option 3 would likely mean undoing the change in #1532.

[Dieterbe]

#1532 makes sure that e.g. a metric with lastUpdate=x-3600 is included in the response to a query from: x to: y, if update-interval is 2*3600 (because if the MT instance started after the last point was sent for a metric, the metric may have been updated after x-3600 but the lastUpdate would not have been saved to the persistent index. If the MT instance is not "fresh" and the metric is still alive, iow if it has ingested points for the metric with newer timestamp values, than the lastUpdate field in the memory index is perfectly accurate, and the #1532 tweak is needlessly "eager".

I think I understand now what you meant with option 3. The above assumes that the lastUpdate field is retained from cassandra/bigtable when loading into memory. But you're suggesting to increment the live, in memory lastUpdate property by update-interval. I think this will work indeed, and involves undoing #1532

The main concern I have is whether there's an edge case where repeated restart cycles will cause the lastUpdate to keep increasing. That would require updating the "adjusted lastUpdate" to the persistent index, but i have gone through the AddOrUpdate code and it seems this wouldn't happen precisely because we always wait update-interval before saving any update.

[Dieterbe]

and would support the large number of Find style operations that have been added since

TagQuery, basically?

[shanson7]

The main concern I have is whether there's an edge case where repeated restart cycles will cause the lastUpdate to keep increasing. That would require updating the "adjusted lastUpdate" to the persistent index, but i have gone through the AddOrUpdate code and it seems this wouldn't happen precisely because we always wait update-interval before saving any update.

Right, we only write updates when updates are received, so I think we should be safe from this scenario.

TagQuery, basically?

Yeah, the various functions that take a TagQuery.

[Dieterbe]

I think I understand now what you meant with option 3. The above assumes that the lastUpdate field is retained from cassandra/bigtable when loading into memory. But you're suggesting to increment the live, in memory lastUpdate property by update-interval. I think this will work indeed, and involves undoing #1532

the upside from this is that we also remove the "needlessly eagerness" of #1532, by only adjusting lastUpdate when it makes sense, but in the average case (with a MT instance being up for a while) lastUpdate will always be precise (because the in memory index always updates its lastUpdate values) and #1532 needlessly bumped the values.

Let me whip up a PR for you...