Statestore prototype#14079
Conversation
There was a problem hiding this comment.
exported var ErrResourceInUse should have comment or be unexported
ed3606b to
9651bbc
Compare
90565d9 to
d8c36de
Compare
filebeat/input/v2/statestore/op.go
Outdated
There was a problem hiding this comment.
I should it more often, especially on a beta in progress feature :)
There was a problem hiding this comment.
I've never used finalizer in Golang before, I've used them in other languages. But after reading a bit more about them and in the context of usage we want to protect the storage as much possible of system and programmer errors.
There was a problem hiding this comment.
Developers are still asked to unlock used resources. The finalizer is just a safety-net.
There was a problem hiding this comment.
I was thinking maybe a Lock that accept a deadline, but we can do the same logic in the consumer with the tryLock().
There was a problem hiding this comment.
TryLock is supposed to return immediately. I don't want to change Lock/Unlock as is, but some LockWithContext would indeed be nice. E.g. we could unblock the call on shutdown.
Idea of TryLock is similar to current 'Finished' flag in filebeat. Assumed code pattern:
res, ok := store.TryLock(path)
if !ok {
return fmt.Errorf("file %v is collected by another input", path)
}
There was a problem hiding this comment.
Sound good, The current API allow for that flexibility so lets keep it simple and add if needed or event make function that wrap that behavior.
There was a problem hiding this comment.
I that case, I think in this case the implementation recommendation would be to fetch entry merge fields and update in the lock?
There was a problem hiding this comment.
A resource is just a 'key' for the store. But the key should be associated with some real-world resource like a file. Users wanting to collect a file must lock said file and only release the lock if the file should not be collected anymore.
The lock must be held for as long as the file is owned by the current process.
Like:
res, ok := store.TryLock(path)
if !ok {
return <error>
}
defer res.Unlock()
// update file meta:
meta := struct{
Path string
LastOpen time.Time
...
}{ path, time.Now(), ... }
res.Update(&meta)
for { <tail file> }
There was a problem hiding this comment.
In the file collection use-case a resource might be updated by a file watcher and an active reader concurrently. This is perfectly valid, as the input holds the lock. In order to have a safe-update, the different go-routines must never update the same set of fields. The file watcher should update file metadata only (e.g. file is renamed), while the active reader should only update the offset. If go-routines adhere to updating distinct set of fields, there is no race.
Unit tests will definitely come. As the interfaces of this layer is still somewhat in the flux I'd like to postpone these to a later PR. I'm planning to first stabilize all interfaces in the different layers + introduce some test support with reusable mocks for the registry itself (so different dependents can be tested more easily). |
kvch
left a comment
There was a problem hiding this comment.
I have posted a few comments. Are you going to add the remove functionality of the store in this PR?
No. |
ef8c2be to
40bed88
Compare
Connector directly wraps a registry and gives access to many stores. Each store accessed will create a session. The registry.Store will be closed once all sessions are closed. Go routines can create pending update operations that will be applied asynchronously later to the persistent registry store. At the same time a go routine can close a store instance if the store is not needed anymore. So to keep the store alive for as long as is required to finally sync the in memory and on-disk state, we introduce the concept of store sessions. A session is active for as long as the Store instance is alive or there are any pending updates in the system. The underlying persistent store can be closed only once all sessions for said store are have been finished.
aa4dfe5 to
773556b
Compare
* State store prototype * Add statestore close and deactivate support * Introduce connector and store sessions Connector directly wraps a registry and gives access to many stores. Each store accessed will create a session. The registry.Store will be closed once all sessions are closed. Go routines can create pending update operations that will be applied asynchronously later to the persistent registry store. At the same time a go routine can close a store instance if the store is not needed anymore. So to keep the store alive for as long as is required to finally sync the in memory and on-disk state, we introduce the concept of store sessions. A session is active for as long as the Store instance is alive or there are any pending updates in the system. The underlying persistent store can be closed only once all sessions for said store are have been finished. * minor cleanups * review * vendor missing dependency
Requires: #14144
The change introduces the statestore package that provides some coordinated access to the registry file.
The store does not implement removal yet. I'm thinking to rather have some GC based approach. This would make it easier to clean stale entries of configs that have gone. Well, and I didn't want the PR to grow too much :)
Go doc: