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

Dynamically change a udp multicast peer's read buffer #99

Conversation

sergiu128
Copy link
Collaborator

@sergiu128 sergiu128 commented Aug 17, 2023

Consider the following scenario: (showcased in the test)

  • exchange sends data on two multicast groups A and B
  • we have 2 udp peers, one for A and one for B which are scheduled to read data
  • these 2 udp peers must share the same read buffers. Initially they are both scheduled to read into b1.

If peer A reads something in b1, peer B must not overwrite what has been read by peer A. Instead, peer B must complete its read into whatever buffer peer A now reads into, say b2. That means peer A must change the buffer peer B reads into in its read callback. To summarize:

  • peer A and B schedule an async read into b1
  • peer A reads and processes what is in b1 (in its async callback)
  • peer B did not read yet. It must not read into b1
  • peer A schedules itself to read into b2 and also changes peer B to read into b2 through B.SetAsyncReadBuffer (in peer A's read callback)
  • by the end of peer A's read, both peer A and peer B will be scheduled to read into b2.

More concretely, for edxm historical feed b1, b2 etc are chunks obtained from a bip buffer.

@sergiu128 sergiu128 merged commit 4204968 into master Aug 17, 2023
@sergiu128 sergiu128 deleted the sergiumarin/CT-4041-sonic-multicast-allow-read-buffer-to-be-changed-in-a-scheduled-async-read branch August 17, 2023 14:50
Comment on lines +2043 to +2045
for _, reader := range readers {
reader.SetAsyncReadBuffer(b)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this indeed needs to be on every AsyncRead when having multiple readers, then might be a good idea to introduce a peers manager that provides you with an AsyncRead that already takes care of this

Copy link
Collaborator Author

@sergiu128 sergiu128 Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed! I think this functionality should be in ava:gateway/lib though such that multiple exchanges can use it without worrying about this detail. I'll make it part of this PR

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.

3 participants