Skip to content

Conversation

@ysarig75
Copy link
Contributor

No description provided.

@ysarig75 ysarig75 requested a review from a team as a code owner May 13, 2025 19:45
tulshi
tulshi previously approved these changes May 13, 2025
@tulshi tulshi dismissed their stale review May 13, 2025 19:56

Pressed approve by accident

@tulshi tulshi requested review from FragLegs, derrumbe and tulshi May 13, 2025 19:56
@FragLegs
Copy link
Contributor

This just occurred to me - the Transmitter might choose to drop events regardless of the stream status. Should we move this language elsewhere?

@ysarig75 ysarig75 force-pushed the clarify-transmitter-pause branch from fb538bd to 0ec9daa Compare May 13, 2025 20:02
@tulshi
Copy link
Contributor

tulshi commented May 13, 2025

This just occurred to me - the Transmitter might choose to drop events regardless of the stream status. Should we move this language elsewhere?

I'd rather be very specific about when this can happen. AFAIK this can happen when the Receiver doesn't poll frequently enough or the stream is paused. Any other condition? If so, we can just add the same / similar language to the poll delivery section.

@ysarig75
Copy link
Contributor Author

This just occurred to me - the Transmitter might choose to drop events regardless of the stream status. Should we move this language elsewhere?

I'd rather be very specific about when this can happen. AFAIK this can happen when the Receiver doesn't poll frequently enough or the stream is paused. Any other condition? If so, we can just add the same / similar language to the poll delivery section.

AFAIK, we don't have a mention of polling logic anywhere in the spec (i.e., we mention polling and pushing with information about how to configure them but we don't specify anything about the polling behavior). So if we want to add language about the polling behavior, where should we add it in the spec?

@FragLegs
Copy link
Contributor

This just occurred to me - the Transmitter might choose to drop events regardless of the stream status. Should we move this language elsewhere?

I'd rather be very specific about when this can happen. AFAIK this can happen when the Receiver doesn't poll frequently enough or the stream is paused. Any other condition? If so, we can just add the same / similar language to the poll delivery section.

I can think of a number of other cases:

  • Transmitter pushes the event and gets an error code from the Receiver
  • Receiver polls frequently but never acks one or more of the SETs
  • The event source overwhelms the Transmitter's ability to deliver events (poll or push)
  • The Receiver changes the event types requested or the subjects on the stream while an event has not yet been delivered

@tulshi
Copy link
Contributor

tulshi commented May 13, 2025

This just occurred to me - the Transmitter might choose to drop events regardless of the stream status. Should we move this language elsewhere?

I'd rather be very specific about when this can happen. AFAIK this can happen when the Receiver doesn't poll frequently enough or the stream is paused. Any other condition? If so, we can just add the same / similar language to the poll delivery section.

I can think of a number of other cases:

  • Transmitter pushes the event and gets an error code from the Receiver
  • Receiver polls frequently but never acks one or more of the SETs
  • The event source overwhelms the Transmitter's ability to deliver events (poll or push)
  • The Receiver changes the event types requested or the subjects on the stream while an event has not yet been delivered

true, so perhaps we can move the language to the Event Delivery section? We should make a new section before the "Push delivery" section, which can talk about conditions under which the Transmitter may drop events

@tulshi
Copy link
Contributor

tulshi commented May 13, 2025

@ysarig75 please also bump up the version number and date.

@ysarig75
Copy link
Contributor Author

This just occurred to me - the Transmitter might choose to drop events regardless of the stream status. Should we move this language elsewhere?

I'd rather be very specific about when this can happen. AFAIK this can happen when the Receiver doesn't poll frequently enough or the stream is paused. Any other condition? If so, we can just add the same / similar language to the poll delivery section.

I can think of a number of other cases:

  • Transmitter pushes the event and gets an error code from the Receiver
  • Receiver polls frequently but never acks one or more of the SETs
  • The event source overwhelms the Transmitter's ability to deliver events (poll or push)
  • The Receiver changes the event types requested or the subjects on the stream while an event has not yet been delivered

true, so perhaps we can move the language to the Event Delivery section? We should make a new section before the "Push delivery" section, which can talk about conditions under which the Transmitter may drop events

Not all the reasons why an event may not be delivered are the same.

  • Transmitter pushes the event and gets an error code from the Receiver: In this case, the transmitter tried to deliver and it failed. There are many possible ways how the failure can be handled (dropping the event, retrying N times and then dropping the event, retrying with a back off, pausing the stream and reaching out to the receiver in unspecified way to triage the issue).
  • The event source overwhelms the Transmitter's ability to deliver events (poll or push): The transmitter may not have the missing events in this case and it is more of an implementation/operation issue
  • The Receiver changes the event types requested or the subjects on the stream while an event has not yet been delivered: Is this really an issue? The receiver explicitly asked to change which events are being sent to it so it doesn't want the extra events. It will be the same if the receiver disable a stream while there were events in processing by the transmitter

So maybe we should only address pause and poll issues in this PR.

@ysarig75 ysarig75 force-pushed the clarify-transmitter-pause branch from 0ec9daa to 3f3253e Compare May 13, 2025 22:20
@ysarig75 ysarig75 force-pushed the clarify-transmitter-pause branch from 3f3253e to 9b5fe5e Compare May 14, 2025 02:34
@ysarig75
Copy link
Contributor Author

@FragLegs / @atultulshi What is the next step for this PR?
I can see the following options:

  1. Only address the pause issue and merge the PR as is (as we need to address the larger issue anyway in the future)
  2. Add some additional language to address the polling issue
  3. Something else?

@tulshi
Copy link
Contributor

tulshi commented May 14, 2025

I've approved. We need one more.

@tulshi tulshi merged commit afa0c3c into openid:main May 15, 2025
2 checks passed
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