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

fix(DA): Sampling behaviour lost requests #1143

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bacv
Copy link
Member

@bacv bacv commented Mar 14, 2025

1. What does this PR implement?

It was observed that even in the perfect network conditions some sampling requests wouldn't be sent. There were couple of cases that were not handled as sampling behavior doesn't necessary start the stream between peers:

  • When there is no connection between peers open_stream returns ConnectionReset io error, sample request fails.
  • When a stream is being reused, receiving end already has stopped listening to the stream after the first message - sample request fails.
  • UnexpectedEof happens when peer closes the stream and in our case this is expected and shouldn't be propagated as an error.

Since dispersal service now has an option to try sampling before publishing into the mempool, a small change was made in sampling service to ensure that sampling service invalidates only blobs that it was requested to sample.

One other change might not be very obvious, but receiving end now tries to listen to the incoming stream as long as it gets the close Eof signal from remote peer. Outgoing and Incoming tasks are put into the same tasks queue.

2. Does the code have enough context to be clearly understood?

3. Who are the specification authors and who is accountable for this PR?

4. Is the specification accurate and complete?

5. Does the implementation introduce changes in the specification?

Checklist

  • 1. Description added.
  • 2. Context and links to Specification document(s) added.
  • 3. Main contact(s) (developers and specification authors) added
  • 4. Implementation and Specification are 100% in sync including changes. This is critical.
  • 5. Link PR to a specific milestone.

@bacv bacv force-pushed the da-sampling-behaviour-errors branch from e30d3ab to a357d87 Compare March 14, 2025 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant