batch plugin logs on postgres #486
Conversation
There was a problem hiding this comment.
Very nice! Just two thoughts:
- even though we flush every second, perhaps for posterity it makes sense to also flush this buffer in teardown/shutdown mode? it's yet another stream/buffer/service/etc, so might make sense to control in a similar way...
- it looks like you're always inserting to postgres, even if kafka is enabled.
|
@mariusandra re yeah that'd make sense |
|
@Twixes thread-level batching now |
|
friendly ping @Twixes |
There was a problem hiding this comment.
Looks like this works so if unbatched Postgres log insertion is a hair-on-fire problem, it's okay, but still has this thing where Kafka logs are now batched two-fold – first via addLog and then via queueMessage. Not ideal since that only increases the time in which a log entry gets into ClickHouse, without benefits. Basically ideally createPluginLogEntries should queue a Kafka message (which it already does) or queue a Postgres row, instead of LogsBuffer creating an additional buffer layer above createPluginLogEntries.
|
That's a good point - |
|
done @Twixes |
|
apologies for the lazy updates on the existing system instead of just rearchitecting in the first place :D |
src/worker/plugins/teardown.ts
Outdated
| server.instanceId | ||
| ) | ||
| source: PluginLogEntrySource.System, | ||
| type: PluginLogEntryType.Info, |
There was a problem hiding this comment.
great catch! probably lingered from a copy paste
) * batch plugin logs on postgres * remove comment * batch at the thread level * fixes * await logs flushing in e2e tests * update tests * flush immediately on tests * fix vm tests * update setupPlugin tests * immediately add to kafka queue if available * rearchitect * Make code subjectively a bit cleaner in a few places * info -> error Co-authored-by: Michael Matloka <dev@twixes.com>
Changes
Addresses #424 and supersedes #431.
This makes log inserts to Postgres from plugins happen in batches every second. Didn't implement batching for other logs but should be easy to do so later if we feel there's a need. Big improvements from this locally. No slow query warnings, and despite the buffer, the logs actually appear faster (since they're not "stuck in traffic").
Checklist