CRE-362: dynamic batching based on observation sizes #1226
Conversation
mchain0
force-pushed
the
cre-362-dynamic-batching-based-on-observation-sizes
branch
from
ee85ff3 to
41a130e
Compare
| return allExecutionIDs, serialized, err | ||
| } | ||
|
|
||
| func (r *reportingPlugin) Query(_ context.Context, _ ocr3types.OutcomeContext) (types.Query, error) { |
There was a problem hiding this comment.
i'd like to see a benchmark test. i have no intuition about how expensive the proto serialization will be nor how many rounds of trial and error we need.
that then turns into a concern about the performance budget of repeated serializations
There was a problem hiding this comment.
good point. algo I made is O(log2(n)), so e.g. 10^6 reports ends up with max 20 (~19.93) optimization rounds (I included that in one of the unit-tests); considering it's a simple structure -- serializing it few times should not be an issue; I agree to empirically verify how many rounds becomes "too slow" -- solidify it with a bench test and warn logs (when closing up to a time limit).
| } | ||
| size := len(serialized) | ||
|
|
||
| r.lggr.Debugw( |
There was a problem hiding this comment.
nit: I'd avoid logging on each iteration, might be too noisy
|
|
||
| r.lggr.Debugw("Query complete", "len", len(serializedBatch), "allExecutionIDs", allExecutionIDs) | ||
|
|
||
| return serializedBatch, nil | ||
| } | ||
|
|
||
| func (r *reportingPlugin) Observation(ctx context.Context, outctx ocr3types.OutcomeContext, query types.Query) (types.Observation, error) { |
There was a problem hiding this comment.
apply also to observation and outcome
CRE-362