writecache: Improve flushing scheme for badger #641
No reviewers
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#641
Loading…
Reference in a new issue
No description provided.
Delete branch "acid-ant/frostfs-node:bugfix/568-imprv-badger-flush"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Close #568
Signed-off-by: Anton Nikiforov an.nikiforov@yadro.com
47cfd47377
tobcb90c455b
@ -23,1 +23,4 @@
flushCh chan *objectSDK.Object
// scheduled4Flush contains objects scheduled for flush via flushCh
// helps to avoid multiple flushing of one object
scheduled4Flush map[oid.Address]any
If it is a set, it is better to use
struct{}
, notany
, asstruct{}
has zero size and thus could be optimized by the compiler.Thanks, updated.
@ -38,0 +58,4 @@
if kv.StreamDone {
return nil
}
if c.scheduled >= flushBatchSize {
Flush batch size was there in
bbolt
, because longView
transactions prevented database from growing in size.In badger we may not need this.
The reason I decided to continue to use it is to allow changing mode for writecache. If wc will be under pressure and for any reason we need to change mode, this will be possible once all objects were scheduled. It may take some time, external call may fail by timeout.
Is It acceptable for us? Or it is better to interrupt some time?
@ -38,0 +62,4 @@
c.cancel()
return nil
}
if got, want := len(kv.Key), len(cid.ID{})+len(oid.ID{}); got != want {
len(internalKey{})
?Yeah, this is better, fixed.
@ -38,0 +70,4 @@
c.processed++
obj := objectSDK.New()
var val []byte
val = append(val, kv.Value...)
val := bytes.Clone
(orslice.Copy
)?Updated, I've chosen append because badger uses it in
CopyValue
.@ -38,0 +74,4 @@
if err = obj.Unmarshal(val); err == nil {
addr := objectCore.AddressOf(obj)
c.cache.scheduled4FlushMtx.RLock()
_, ok := c.cache.scheduled4Flush[addr]
Correct me if I am wrong: the purpose of this map is to prevent flushing the same object twice?
Right, It is possible that a background routine which storing data may hang, and on the next iteration the same object will be scheduled. When using stream it is impossible to start from one of the key, no such api. Also, retrieved keys sorted lexicographically, not in putting in db order.
@ -38,0 +91,4 @@
return nil
}
} else {
c.cache.log.Debug(fmt.Sprintf("error unmarshal: %s", err))
Warn
orError
(and probably increase shard error counterlog.Error(logs.Message, zap.Error(err))
Actually, if we increase error counter, there is no need to log.
Agree, removed log entry.
@ -129,3 +146,1 @@
c.modeMtx.RUnlock()
return
}
ctx, cancel := context.WithCancel(context.TODO())
Background
? Or provide context toflushSmallObjects
?Think it is better to use global context in a separate task #642. Requires a lot of refactoring, because we need to change the signature of
Init
method.@ -134,2 +151,2 @@
if count == 0 {
c.modeMtx.RUnlock()
stream := c.db.NewStream()
// Logic within Send method can expect single threaded execution.
can expect
orexpects
?All calls to
Send
are done by a single goroutine.@ -136,0 +151,4 @@
stream := c.db.NewStream()
// Logic within Send method can expect single threaded execution.
stream.Send = coll.Send
if err := stream.Orchestrate(ctx); err != nil {
Does it exit after all object in the database were streamed through?
Yes, in opposite to
Subscribe
.@ -141,3 +162,2 @@
c.log.Debug(logs.WritecacheTriedToFlushItemsFromWritecache,
zap.Int("count", count),
zap.String("start", base58.Encode(lastKey[:])))
zap.Int("scheduled", coll.scheduled), zap.Int("processed", coll.processed))
GC interval is 1 minute by default currently. It may be beneficial to run it after each flush cycle instead. What do you think? @TrueCloudLab/storage-core-committers @TrueCloudLab/storage-core-developers
If you mean badgers gs, I think that will be useful here if we scheduled some objects for deletion. We're flushing each second, could this be a problem?
I don't think it must be some correlation between badger GC and flush.
bcb90c455b
to4c4968f95d
4c4968f95d
to7a3c157537
@ -136,0 +153,4 @@
"error during flushing object from wc: %s", err))
}
c.modeMtx.RUnlock()
if coll.scheduled == 0 {
Please explain this line. I don't understand why the flush ends if no objects are scheduled?
It is possible that few objects still exists in db and at the same time scheduled for flush.
To prevent iteration over already scheduled objects, we need to check for scheduled counter, not for processed.
This loop running by timer each second.
7a3c157537
to1129564e7a