cilium / statedb

In-memory state database for Go
Apache License 2.0
35 stars 2 forks source link

`reconciler.BatchOperations` provides object revision, where `reconciler.Operations` does not #56

Open gandro opened 2 months ago

gandro commented 2 months ago

Consider the reconciler operations interfaces (as of v0.2.x):

type Operations[Obj any] interface {
    Update(ctx context.Context, txn statedb.ReadTxn, obj Obj) error
    Delete(context.Context, statedb.ReadTxn, Obj) error
    Prune(context.Context, statedb.ReadTxn, statedb.Iterator[Obj]) error
}

type BatchEntry[Obj any] struct {
    Object   Obj
    Revision statedb.Revision
    Result   error

    // ... unexported fields
}

type BatchOperations[Obj any] interface {
    UpdateBatch(ctx context.Context, txn statedb.ReadTxn, batch []BatchEntry[Obj])
    DeleteBatch(context.Context, statedb.ReadTxn, []BatchEntry[Obj])
}

Implementors of the batch are able to obtain the object revision, where as implementers of the single operations interface are not. Since the Operations interface is mandatory, clients might want to implement part of the Operations interface on top of the BatchOperations interface (basically issuing batches containing a single element). But this is currently not possible, because invoking BatchOperations requires a revision not given to the Operations implementation.

Thus, this issue suggests that the Operations methods also get a StateDB revision for each passed object.

joamaki commented 1 month ago

Agree with Operations also getting the revision. Let's accumulate some more API changes and release this as part of v0.4.

The BatchOperations interface I'm not too happy about as there's fair bit of allocations going on. This is why I also didn't do batch ops in pkg/bpf/ops_linux.go as there wasn't too much benefit to using a BPF batch op due to the inefficiency of the interface (less sys CPU time spent though...). I think I would like to revisit its design and see if we can make it work nicely with BPF map batch updates with minimal copying and allocations. Perhaps using iter.Seq2[Obj, *Result] to avoid the []BatchEntry slice allocation and on the BPF ops side having largeish buffers in sync.Pool that we reuse.