influxdata / influxdb

Scalable datastore for metrics, events, and real-time analytics
https://influxdata.com
Apache License 2.0
28.8k stars 3.55k forks source link

Connect last cache to catalog at runtime #25169

Closed hiltontj closed 2 months ago

hiltontj commented 3 months ago
pauldix commented 3 months ago

You can probably just save the whole Catalog on creation/deletion as that's unlikely to be that frequent of an operation. Only reason to put it into the WAL instead is to batch up a bunch of changes before doing the Catalog save.

hiltontj commented 3 months ago

You can probably just save the whole Catalog on creation/deletion as that's unlikely to be that frequent of an operation. Only reason to put it into the WAL instead is to batch up a bunch of changes before doing the Catalog save.

If I understand correctly, I think that is what I am doing in #25170 (still in draft as I need to add tests etc.).

On create, it clones the database schema from the catalog, updates and replaces it, which increments the sequence number but doesn't explicitly interact with the WAL (see here). I used similar logic on delete.

pauldix commented 3 months ago

So if you don't actually persist the updated Catalog to object store, then there's a scenario where this last cache configuration information could be lost. Currently, the catalog gets persisted when a segment gets persisted, only if it has been updated.

So if you set the in-memory structure, but don't have a corresponding entry in the WAL, you'll lose that update if the server crashes or gets restarted before it has a chance to persist the catalog.

My suggestion was to force persistence of the catalog (if the cache is new/deleted) before you return a success to the user. This ensures that the Catalog will have the last cache info even in crash/restart scenarios.

hiltontj commented 2 months ago

Right, understood, the way I'm doing it now without a WAL entry would be super problematic. I will try to go that route then to force persistence.

Do you think with the WAL refactoring work (https://github.com/influxdata/influxdb/issues/25142) we could more easily accommodate catalog updates like this with a WAL op/entry?

pauldix commented 2 months ago

You could make it a WalOp. With the refactor, there's just a single wal file with a bunch of operations that get applied at once (default of 1s total operations). And periodically a snapshot will be run (default 10m), which is the point at which the Catalog would be persisted.

The thing there is that I don't have the write operations sequenced against other ops. The writes get combined together as they come in, into their own tables & chunks. So they're ordered within the context of a chunk.

What this means is that other types of ops (like adding last caches to the catalog) could go in the wal, but they'd apply to whatever second they came in at. So we'd probably apply those ops, then apply the writes.

I don't think that matters in this case, but it's something to consider if we use the wal for other operations. Essentially, there's a sequence for the files, but within a file (i.e. a second of operations) they're not really sequenced. That could be changed later, but it was more efficient for now to not have that extra metadata and information in the wal.

hiltontj commented 2 months ago

What this means is that other types of ops (like adding last caches to the catalog) could go in the wal, but they'd apply to whatever second they came in at. So we'd probably apply those ops, then apply the writes.

With the 1s interval, I guess its possible, but it seems unlikely that the config ops order with respect to the write ops would cause a problem - so applying them before writes would probably be fine. For example, you would have to have the write op that creates/modifies the schema and the config op that adds the cache land within the same second for the ordering to matter. Obviously, it would be ideal if their ordering was captured, but that could be part of a later effort.

I was able to get the forced persistence of the catalog to work - I still need to see that it works when the system is under write load - but my hope for now is to move forward without introducing the complexity of a new WalOp.

pauldix commented 2 months ago

Sounds good. Good point on applying the last cache thing before writes that apply schema changes. I'll have to give more thought to how the catalog updates work with the new wal structure.

hiltontj commented 2 months ago

I just opened https://github.com/influxdata/influxdb/issues/25171, which I discovered while testing this in #25170.

hiltontj commented 2 months ago

I opened https://github.com/influxdata/influxdb/issues/25172 to explore using the WAL for configuration changes (specifically w.r.t. the last cache).

Edit: https://github.com/influxdata/influxdb/issues/25173 is relevant.