fluent / fluent-bit-go

Fluent Bit Golang package to build plugins
Apache License 2.0
189 stars 52 forks source link

output: make plugin context operations safe for concurrent use #46

Closed l2dy closed 2 years ago

l2dy commented 3 years ago

See #45.

nokute78 commented 3 years ago

How about using a plane map + RWMutex ?

https://pkg.go.dev/sync#Map

The Map type is specialized. Most code should use a plain Go map instead,
 with separate locking or coordination, for better type safety and to make it easier to maintain other invariants along with the map content.

The Map type is optimized for two common use cases:
 (1) when the entry for a given key is only ever written once but read many times, as in caches that only grow,
 or (2) when multiple goroutines read, write, and overwrite entries for disjoint sets of keys.
 In these two cases, use of a Map may significantly reduce lock contention compared to a Go map paired with a separate Mutex or RWMutex.
l2dy commented 3 years ago

Output plugins should be heavy on reading the contexts map, not updating it. If context state update is needed in FLBPluginFlushCtx, store a pointer to the mutable state in context and it will be out of the scope of this sync.Map.

Only FLBPluginInit has access to the plugin struct, and every call assigns a new key.

l2dy commented 3 years ago

In fact, every time FLBPluginSetContext is called a new key is used, so this is exactly use case (1), a cache that only grows.

nokute78 commented 3 years ago

OK. I understood FLBPluginSetContext doesn't support overwriting. Thank you.

l2dy commented 3 years ago

@edsiper Could you review this?

l2dy commented 2 years ago

@nokute78 Can you merge this pull request?

nokute78 commented 2 years ago

@l2dy @edsiper Sorry I can't merge since I don't have a permission to merge for this repo.

This PR is LGTM.

edsiper commented 2 years ago

@nokute78 pls check for permissions now...

nokute78 commented 2 years ago

Oh I can find "Merge pull request" button now.