Skip to content

Commit 895dda3

Browse files
remove snapshot from puller
1 parent 5b3c542 commit 895dda3

4 files changed

Lines changed: 72 additions & 189 deletions

File tree

a2asrv/eventqueue/pull_event_queue_impl.go

Lines changed: 3 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,6 @@ type Puller interface {
5050
// PullerProvider is a function that returns a puller for the given task ID.
5151
type PullerProvider func(ctx context.Context, taskID a2a.TaskID) (Puller, error)
5252

53-
// NewStaticPullerProvider returns a PullerProvider that always returns the same Puller.
54-
func NewStaticPullerProvider(es Puller) PullerProvider {
55-
return func(ctx context.Context, taskID a2a.TaskID) (Puller, error) {
56-
return es, nil
57-
}
58-
}
59-
6053
// PullConfig configures the behavior of a pull-based event queue manager.
6154
type PullConfig struct {
6255
// PollInterval is the interval at which the puller is polled for new events.
@@ -65,9 +58,6 @@ type PullConfig struct {
6558
// InactivityTimeout is the duration of inactivity after which the reader will time out.
6659
// Defaults to 5 minutes. Set to 0 to disable inactivity timeout.
6760
InactivityTimeout time.Duration
68-
// AccessCheck is an optional callback executed before emitting the initial snapshot
69-
// of a task to verify that the calling context has permission to access the task.
70-
AccessCheck func(context.Context, *a2a.Task) error
7161
// OnInactivity is an optional callback function that is called when a task has exceeded the
7262
// InactivityTimeout. It's only triggered from Reader.Read().
7363
// The returned task is used to update the snapshot.
@@ -124,12 +114,7 @@ func (m *pullQueueManager) CreateReader(ctx context.Context, taskID a2a.TaskID)
124114
return nil, fmt.Errorf("failed to get puller: %w", err)
125115
}
126116

127-
snapshot, err := getSnapshot(ctx, puller, taskID)
128-
if err != nil {
129-
return nil, fmt.Errorf("failed to get snapshot: %w", err)
130-
}
131-
132-
return newPullReader(puller, taskID, m, snapshot), nil
117+
return newPullReader(puller, taskID, m), nil
133118
}
134119

135120
// CreateWriter implements Manager.CreateWriter. It delegates to the in-memory manager.
@@ -146,21 +131,17 @@ var _ Reader = (*pullReader)(nil)
146131

147132
type pullReader struct {
148133
taskID a2a.TaskID
149-
snapshot *Message
150134
puller Puller
151135
manager *pullQueueManager
152136
eventsChan chan *Message
153137
closed chan struct{}
154138
ctxCancel context.CancelFunc
155-
156-
emittedSnapshot bool
157139
}
158140

159-
func newPullReader(p Puller, taskID a2a.TaskID, queueManager *pullQueueManager, snapshot *Message) *pullReader {
141+
func newPullReader(p Puller, taskID a2a.TaskID, queueManager *pullQueueManager) *pullReader {
160142
ctx, cancel := context.WithCancel(context.Background())
161143
reader := &pullReader{
162144
taskID: taskID,
163-
snapshot: snapshot,
164145
puller: p,
165146
manager: queueManager,
166147
eventsChan: make(chan *Message),
@@ -215,10 +196,6 @@ func (r *pullReader) dispatchMessages(ctx context.Context, resp *PullResponse) s
215196
if msg == nil || msg.Event == nil {
216197
continue
217198
}
218-
// Snapshot is emitted directly from r.snapshot by Read, so filter out to not send duplicate.
219-
if _, isTask := msg.Event.(*a2a.Task); isTask {
220-
continue
221-
}
222199
select {
223200
case r.eventsChan <- msg:
224201
case <-ctx.Done():
@@ -237,17 +214,6 @@ func (r *pullReader) dispatchMessages(ctx context.Context, resp *PullResponse) s
237214
// If the inactivity timeout is reached, it will trigger the OnInactivity callback if configured,
238215
// and return ErrInactivityTimeout.
239216
func (r *pullReader) Read(ctx context.Context) (*Message, error) {
240-
if !r.emittedSnapshot {
241-
if err := r.accessCheck(ctx); err != nil {
242-
return nil, err
243-
}
244-
r.emittedSnapshot = true
245-
return r.snapshot, nil
246-
}
247-
if taskupdate.IsFinal(r.snapshot.Event) {
248-
return nil, ErrQueueClosed
249-
}
250-
251217
var timeout <-chan time.Time
252218
if r.manager.cfg.InactivityTimeout > 0 {
253219
timer := time.NewTimer(r.manager.cfg.InactivityTimeout)
@@ -269,8 +235,7 @@ func (r *pullReader) Read(ctx context.Context) (*Message, error) {
269235
return nil, fmt.Errorf("%w: failed to call inactivity callback:%w", ErrInactivityTimeout, err)
270236
}
271237
if task != nil {
272-
r.snapshot = newMessage(task)
273-
r.emittedSnapshot = false
238+
return newMessage(task), nil
274239
}
275240
}
276241
return nil, fmt.Errorf("%w after %v", ErrInactivityTimeout, r.manager.cfg.InactivityTimeout)
@@ -284,52 +249,6 @@ func (r *pullReader) Close() error {
284249
return nil
285250
}
286251

287-
func (r *pullReader) accessCheck(ctx context.Context) error {
288-
if r.manager.cfg.AccessCheck == nil {
289-
return nil
290-
}
291-
task, ok := r.snapshot.Event.(*a2a.Task)
292-
if !ok {
293-
return fmt.Errorf("snapshot event is not a task: %T", r.snapshot.Event)
294-
}
295-
return r.manager.cfg.AccessCheck(ctx, task)
296-
}
297-
298-
func getSnapshot(ctx context.Context, puller Puller, taskID a2a.TaskID) (*Message, error) {
299-
resp, err := puller.Pull(ctx, taskID, nil)
300-
if err != nil {
301-
closePullerOnError(ctx, puller)
302-
return nil, fmt.Errorf("snapshot pull failed for task %v: %w", taskID, err)
303-
}
304-
if resp == nil || len(resp.Messages) == 0 {
305-
closePullerOnError(ctx, puller)
306-
return nil, fmt.Errorf("puller returned no snapshot for task %v", taskID)
307-
}
308-
snapshotMsg := resp.Messages[0]
309-
if snapshotMsg == nil || snapshotMsg.Event == nil {
310-
closePullerOnError(ctx, puller)
311-
return nil, fmt.Errorf("pull queue: puller returned nil snapshot message for task %q", taskID)
312-
}
313-
task, ok := snapshotMsg.Event.(*a2a.Task)
314-
if !ok {
315-
closePullerOnError(ctx, puller)
316-
return nil, fmt.Errorf("pull queue: puller's first message for task %q is %T, want *a2a.Task",
317-
taskID, snapshotMsg.Event)
318-
}
319-
if task.ID != taskID {
320-
closePullerOnError(ctx, puller)
321-
return nil, fmt.Errorf("pull queue: task ID mismatch in snapshot for task %q: got %q",
322-
taskID, task.ID)
323-
}
324-
return snapshotMsg, nil
325-
}
326-
327-
func closePullerOnError(ctx context.Context, puller Puller) {
328-
if err := puller.Close(ctx); err != nil {
329-
log.Warn(ctx, "Error closing puller: %v", err)
330-
}
331-
}
332-
333252
func newMessage(task *a2a.Task) *Message {
334253
return &Message{
335254
Event: task,

a2asrv/eventqueue/pull_event_queue_impl_test.go

Lines changed: 15 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,12 @@ import (
3535
func TestPullQueue_SubscribeToTask(t *testing.T) {
3636
t.Parallel()
3737

38-
mockPullerErr := errors.New("puller error on snapshot fetch")
39-
4038
testCases := []struct {
4139
name string
4240
snapshotFn func(taskID a2a.TaskID) *a2a.Task
4341
eventsFn func(taskID a2a.TaskID) []*eventqueue.Message
4442
wantEventsFn func(taskID a2a.TaskID) []a2a.Event
4543
wantErr error
46-
snapshotErr error
47-
accessCheck func(context.Context, *a2a.Task) error
4844
}{
4945
{
5046
name: "snapshot then final completed event",
@@ -154,46 +150,6 @@ func TestPullQueue_SubscribeToTask(t *testing.T) {
154150
}
155151
},
156152
},
157-
{
158-
name: "snapshot error",
159-
snapshotFn: func(taskID a2a.TaskID) *a2a.Task {
160-
return &a2a.Task{
161-
ID: taskID,
162-
Status: a2a.TaskStatus{State: a2a.TaskStateSubmitted},
163-
}
164-
},
165-
snapshotErr: mockPullerErr,
166-
wantErr: mockPullerErr,
167-
},
168-
{
169-
name: "access check fails",
170-
// remoteSubscription yields the snapshot before the Read loop,
171-
// then AccessCheck rejects on the first Read and prevents subsequent events
172-
// from reaching the subscriber.
173-
snapshotFn: func(taskID a2a.TaskID) *a2a.Task {
174-
return &a2a.Task{
175-
ID: taskID,
176-
Status: a2a.TaskStatus{State: a2a.TaskStateSubmitted},
177-
}
178-
},
179-
eventsFn: func(taskID a2a.TaskID) []*eventqueue.Message {
180-
return []*eventqueue.Message{
181-
{
182-
Event: a2a.NewMessage(a2a.MessageRoleAgent, a2a.NewTextPart("hi")),
183-
TaskVersion: 2,
184-
},
185-
}
186-
},
187-
wantEventsFn: func(taskID a2a.TaskID) []a2a.Event {
188-
return []a2a.Event{
189-
&a2a.Task{ID: taskID, Status: a2a.TaskStatus{State: a2a.TaskStateSubmitted}},
190-
}
191-
},
192-
accessCheck: func(ctx context.Context, task *a2a.Task) error {
193-
return a2a.ErrUnauthorized
194-
},
195-
wantErr: a2a.ErrUnauthorized,
196-
},
197153
}
198154
wantCloseCount := int32(1)
199155
for _, tc := range testCases {
@@ -217,10 +173,8 @@ func TestPullQueue_SubscribeToTask(t *testing.T) {
217173
}
218174

219175
env := setupTest(t, &testEnvOptions{
220-
snapshot: snapshot,
221-
events: events,
222-
snapshotErr: tc.snapshotErr,
223-
accessCheck: tc.accessCheck,
176+
snapshot: snapshot,
177+
events: events,
224178
})
225179
reqHandler := *env.handler
226180
var gotEvents []a2a.Event
@@ -316,20 +270,16 @@ type testEnv struct {
316270
var _ eventqueue.Puller = (*mockPuller)(nil)
317271

318272
type mockPuller struct {
319-
snapshot *a2a.Task
320-
events []*eventqueue.Message
321-
snapshotErr error
322-
closeCount atomic.Int32
273+
snapshot *a2a.Task
274+
events []*eventqueue.Message
275+
closeCount atomic.Int32
323276
}
324277

325-
func newMockPuller(snapshot *a2a.Task, events []*eventqueue.Message, snapshotErr error) *mockPuller {
326-
return &mockPuller{snapshot: snapshot, events: events, snapshotErr: snapshotErr}
278+
func newMockPuller(snapshot *a2a.Task, events []*eventqueue.Message) *mockPuller {
279+
return &mockPuller{snapshot: snapshot, events: events}
327280
}
328281

329282
func (m *mockPuller) Pull(ctx context.Context, taskID a2a.TaskID, cursor eventqueue.PullCursor) (*eventqueue.PullResponse, error) {
330-
if m.snapshotErr != nil {
331-
return nil, m.snapshotErr
332-
}
333283
if cursor == nil {
334284
return &eventqueue.PullResponse{
335285
Messages: []*eventqueue.Message{{Event: m.snapshot, TaskVersion: 1}},
@@ -357,9 +307,7 @@ func (m *mockPuller) Close(ctx context.Context) error {
357307
type testEnvOptions struct {
358308
snapshot *a2a.Task
359309
events []*eventqueue.Message
360-
snapshotErr error
361310
inactivityTimeout time.Duration
362-
accessCheck func(context.Context, *a2a.Task) error
363311
}
364312

365313
func setupTest(t *testing.T, opts *testEnvOptions) *testEnv {
@@ -371,14 +319,13 @@ func setupTest(t *testing.T, opts *testEnvOptions) *testEnv {
371319
if opts.snapshot != nil {
372320
store.WithTasks(t, opts.snapshot)
373321
}
374-
puller := newMockPuller(opts.snapshot, opts.events, opts.snapshotErr)
375-
pp := eventqueue.NewStaticPullerProvider(puller)
322+
puller := newMockPuller(opts.snapshot, opts.events)
323+
pp := newPullerProvider(puller)
376324

377325
pullQueueManager := eventqueue.NewPullQueueManager(pp, eventqueue.PullConfig{
378326
InactivityTimeout: opts.inactivityTimeout,
379327
PollInterval: 5 * time.Millisecond,
380328
UseInMemory: useInMemory,
381-
AccessCheck: opts.accessCheck,
382329
})
383330
wq := workqueue.NewInMemory(nil)
384331
executor := &testexecutor.TestAgentExecutor{}
@@ -397,6 +344,12 @@ func setupTest(t *testing.T, opts *testEnvOptions) *testEnv {
397344
}
398345
}
399346

347+
func newPullerProvider(p eventqueue.Puller) eventqueue.PullerProvider {
348+
return func(ctx context.Context, taskID a2a.TaskID) (eventqueue.Puller, error) {
349+
return p, nil
350+
}
351+
}
352+
400353
func useInMemory(ctx context.Context) bool {
401354
cc, ok := a2asrv.CallContextFrom(ctx)
402355
if !ok {

examples/clustermode/server/deploy/nginx.conf

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ http {
1414
proxy_pass http://backend;
1515
proxy_set_header Host $host;
1616
proxy_set_header X-Real-IP $remote_addr;
17+
18+
# Retry on 502 (non_idempotent unlocks POST retries).
19+
# Handles pods that crash before sending response headers.
20+
proxy_next_upstream error http_502 non_idempotent;
21+
proxy_next_upstream_tries 3;
1722
}
1823
}
1924
}

0 commit comments

Comments
 (0)