Conversation
moskyb
left a comment
There was a problem hiding this comment.
i like the idea here, but given the churn going on with pings at the moment with agent push, i think potentially it might be tricky to get remote config manipulation working through both push and polling ping at the same time.
that said though, i think responding to 429s with increased batch sizes is a great idea, and i'd be happy to merge that side of this PR as-is. perhaps we could bump the batch size limit even further? 1000?
|
My very first thought, without even considering the proposal, is: why not just increase the batch size default, without any dynamic behaviour? Having the batch size be backend-controlled is good in case we discover that particular batch sizes are bad for us. I instinctually disagree with @moskyb: I don't think bumping the batch size dynamically based on 429s is a good idea, even if there is an upper limit. I could write some words on it here but I think there there are probably better approaches. |
yeah, i agree with this. |
Disclosure: Ampcode assisted, human curated.
Summary
This change adds server-driven artifact batching controls to the agent and introduces light adaptive behaviour for artifact creation when rate-limited.
What Changed
artifact_create_batch_sizeartifact_update_batch_size_maxBUILDKITE_ARTIFACT_CREATE_BATCH_SIZEBUILDKITE_ARTIFACT_UPDATE_BATCH_SIZE_MAXartifact uploadfor both knobs.CreateArtifactsbatching logic to:429responses (doubling, capped)UpdateArtifactslogic to chunk large state updates by configured max batch size.429Why
Notes
go tool gofumpt -extra -w ...on modified filesgo test ./api ./agent ./internal/artifact ./clicommand