Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bulk subscription - sidecar crashes after first batch is delivered #6414

Closed
GianvitoDifilippo opened this issue May 26, 2023 · 3 comments · Fixed by #6415
Closed

Bulk subscription - sidecar crashes after first batch is delivered #6414

GianvitoDifilippo opened this issue May 26, 2023 · 3 comments · Fixed by #6415
Assignees
Labels
kind/bug Something isn't working P0
Milestone

Comments

@GianvitoDifilippo
Copy link

GianvitoDifilippo commented May 26, 2023

In what area(s)?

/area runtime

What version of Dapr?

1.11.0-rc.5

Expected Behavior

Using the alpha feature "bulk pub-sub" with maxMessagesCount = 10, the subscriber endpoint is called twice when bulk publishing 20 messages.

Actual Behavior

The first batch of 10 messages is correctly delivered, then the sidecar crashes.

2023-05-26 11:30:01 panic: close of closed channel
2023-05-26 11:30:01 
2023-05-26 11:30:01 goroutine 149 [running]:
2023-05-26 11:30:01 github.com/dapr/dapr/pkg/runtime/pubsub.(*defaultBulkSubscriber).BulkSubscribe.func1.1({0x6e7e8e0?, 0xc0000c6100?})
2023-05-26 11:30:01     /home/runner/work/dapr/dapr/pkg/runtime/pubsub/default_bulksub.go:86 +0x4d
2023-05-26 11:30:01 github.com/dapr/dapr/pkg/runtime/pubsub.flushMessages({0x6ecbc98, 0xc001e979a0}, {0xc0011dc530, 0x9}, {0xc001c42f00, 0xa, 0xa}, 0x0?, 0xc000da1a80)
2023-05-26 11:30:01     /home/runner/work/dapr/dapr/pkg/runtime/pubsub/default_bulksub.go:145 +0x2de
2023-05-26 11:30:01 github.com/dapr/dapr/pkg/runtime/pubsub.processBulkMessages({0x6ecbc98, 0xc001e979a0}, {0xc0011dc530, 0x9}, 0xc001ed5260, {0x0?, 0x0?}, 0x0?)
2023-05-26 11:30:01     /home/runner/work/dapr/dapr/pkg/runtime/pubsub/default_bulksub.go:116 +0x418
2023-05-26 11:30:01 created by github.com/dapr/dapr/pkg/runtime/pubsub.(*defaultBulkSubscriber).BulkSubscribe
2023-05-26 11:30:01     /home/runner/work/dapr/dapr/pkg/runtime/pubsub/default_bulksub.go:62 +0x185

I have looked into this issue, but I'm not sure I'm having the same problem. However, I tried adding this configuration file, but nothing changed.

apiVersion: dapr.io/v1alpha1
kind: Configuration
metadata:
  name: daprConfig
spec:
  tracing:
    samplingRate: "0"

Side note: I tried to use declarative subscriptions as described here but it did not work: the subscription endpoint was called once for each message, so I fell back to the programmatic subscription. This is the configuration I used:

apiVersion: dapr.io/v2alpha1
kind: Subscription
metadata:
  name: messagebus
spec:
  topic: bulk-test
  routes:
    default: /subscribe
  pubsubname: messagebus
  bulkSubscribe:
    enabled: true
    maxMessagesCount: 10
    maxAwaitDurationMs: 40
scopes:
  - subscriber

Steps to Reproduce the Problem

  1. Download the repro
  2. Run docker compose up
  3. Either open http:/localhost:5000/swagger and execute the /publish action or make a POST request with an empty body to http:/localhost:5000/publish
@GianvitoDifilippo GianvitoDifilippo added the kind/bug Something isn't working label May 26, 2023
@yaron2 yaron2 added the P0 label May 26, 2023
@yaron2 yaron2 added this to the v1.11 milestone May 26, 2023
@mukundansundar
Copy link
Contributor

mukundansundar commented May 26, 2023

@GianvitoDifilippo Thanks for reporting this. We are looking into this.
But a quick look into the source code of the Subscriber, shows that Ok() is being returned. For HTTP, the expected format is detailed in the doc here otherwise it will retry to send the event again.

For the panic, we are taking a look at it.

@GianvitoDifilippo
Copy link
Author

Returning the correct response actually solved the problem. I completely overlooked that section of the documentation - my bad, maybe I was too eager to try the new feature :)
Thanks for your help @mukundansundar!

@DeepanshuA
Copy link
Contributor

DeepanshuA commented May 27, 2023

Thanks @GianvitoDifilippo for reporting this issue. It helped us to look into the problem with Bulk Subscribe response which would have happened when App replied back few specific kind of replies.
This seems to have been introduced when we introduced resiliency in Bulk Subscribe.

@dapr/maintainers-dapr @dapr/approvers-dapr I have opened up a PR #6415 to address this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working P0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants