Clean up global variables in safekeeper code
We have at least several places in the code where global variables are used:
- some places call
GlobalTimelines::<funcname>to lookup timelines static TIMELINES_STATEstores all timelines as a global variablestatic REMOTE_STORAGEstores config for remote storage (e.g. S3)
To resolve the issue, we should replace GlobalTimelines:: calls with Arc<GlobalTimelines> struct. It will contain the hashmap of all timelines (the same way TIMELINES_STATE is working now). It should be passed around to the code that needs access to all timelines, instead of everything relying on global access through GlobalTimelines.
- [x] remove all
GlobalTimelines::calls - [x] remove
static TIMELINES_STATE
REMOTE_STORAGE too should be passed around as a Arc<RemoteStorage>. Each task that requires access to remote storage should get Arc<RemoteStorage> as a dependency during initialization.
- [x] remove
static REMOTE_STORAGE
If some background task requires access to all global state (remote storage, timeline, config), we can bundle them together inside Arc<SafekeeperApp>, which is easier to carry around.
Also, currently struct GlobalTimelinesState contains two fields that are part of the global state:
conf: Option<SafeKeeperConf>,
broker_active_set: Arc<TimelinesSet>,
Currently conf is required in many places and usually it just copied from global state on demand. The config is loaded once on startup and never changes. This is wasteful, we should replace it with Arc<SafeKeeperConf> and always carry Arc<..> instead of SafeKeeperConf.
- [x] replace
SafeKeeperConfusages withArc<SafeKeeperConf>everywhere
Why?
To improve testing, to allow to run more than one safekeeper instance in a single process. To simplify creating new dependencies without introducing global variables and special getters.
@petuhovskiy can I take up this issue ?
@petuhovskiy can I take up this issue ?
@fowlerlee Sure, no one is working on this right now. You can start and open a PR, thanks!
We just merged
- https://github.com/neondatabase/neon/pull/10179
@petuhovskiy anything left on this issue?
I tried to grep static in rust safekeeper code, and I don't see anything left, except metrics and jemalloc. So I think we can close this issue.