neondatabase / neon

Neon: Serverless Postgres. We separated storage and compute to offer autoscaling, code-like database branching, and scale to zero.
https://neon.tech
Apache License 2.0
15.3k stars 446 forks source link

consumption_metrics: very old cached values can be sent because of migrations #9032

Open koivunej opened 2 months ago

koivunej commented 2 months ago

Internal slack thread: https://neondb.slack.com/archives/C061CPK7UQL/p1726556769067039?thread_ts=1726493755.104459&cid=C061CPK7UQL

Symptom:

Sent/exported synthetic sizes:

  1. synthetic_size=500GB at t=0
  2. synthetic_size=1000GB at t=10min
  3. synthetic_size=500GB at t=20min

More broadly, what happened:

  1. synthetic_size=1000GB was reported from pageserver-12.us-east-2
  2. as part of deploy, we migrated tenant shard zero to pageserver-11.us-east-2, no cached value
  3. synthetic_size=999GB from pageserver-11.us-east-2
  4. (a week passes)
  5. synthetic_size=500GB from pageserver-11.us-east-2
  6. next deploy, we migrated tenant shard zero back to pageserver-12.us-east-2, cached value was found!
  7. synthetic_size=1000GB from pageserver-12.us-east-2 (assumed cached)
  8. synthetic_size=500GB from pageserver-12.us-east-2 freshly calculated

How to fix it:

koivunej commented 2 months ago

Discussed in meeting: Generation number increasing by at most 1 to reuse cached values is the best filter.

No idea right now: how to store generations in the cache? Cache might not be leaked outside, so perhaps that is easy to do.

jcsp commented 1 month ago

@skyzh can you take a look at this one when you're back in the office?

skyzh commented 1 month ago

I think storing generation would make sense, and I would like to have a new format for consumption_metrics.json (I assume nobody else except pageserver is using this json file).

I'm refactoring the storing/restoring code right now...

skyzh commented 1 month ago

... it seems that for upload_metrics_bucket and upload_metrics_http, we will need to keep the original message format

skyzh commented 1 month ago

otherwise, I think I'll have a PR to migrate to the new format, and another patch to implement the ">= generation => do not report" logic

skyzh commented 3 weeks ago

this week: add generation number to the disk cache and the logic to invalidate cache based on generation number