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

[Access] Improve subscription Streamer functionality to graceful shutdown on demand #5561

Open
Tracked by #6163
Guitarheroua opened this issue Mar 19, 2024 · 2 comments
Assignees
Labels
Preserve Stale Bot repellent S-Access

Comments

@Guitarheroua
Copy link
Contributor

Problem Definition

The Streamer now should handle one more extra block to gracefully shut the subscription (see discussion in PR). There should be a way to send the reply and shut down the subscription immediately after that on demand.

For this, the Streamer, and perhaps other subscription core functionalities, should be improved to react to shutdown signals and handle replies and subscriptions properly.

Definition of Done

  • Implement and update unit and functional tests to ensure that the new functionality works properly and the old one works as previously.
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale Label used when marking an issue stale. label Jun 20, 2024
@peterargue peterargue added Preserve Stale Bot repellent and removed Stale Label used when marking an issue stale. labels Jun 21, 2024
@illia-malachyn
Copy link
Contributor

Options Considered:

1. Add an EOF Boolean Flag to HeightBasedSubscription and Pass it to getData()

The idea is to introduce an EOF flag in the HeightBasedSubscription that would be set when a specific condition is met and then passed to the getData() function.

Drawbacks:

  • Requires refactoring all getData() callbacks, even though only one would actually use the flag.
  • Most subscriptions do not need an EOF flag, making this approach unnecessary for the majority of cases.
  • Significantly increases the complexity of the subscription package, making data flow more difficult to understand and maintain.

2. Return an Error Alongside the Response in getData() (e.g., ErrSubscriptionShouldBeClosed)

This approach involves returning a specific error to indicate that the subscription should be closed and data fetching can be stopped.

Advantages:

  • Limits refactoring to a small number of functions within the subscription package.

Drawbacks:

  • Can be confusing because it involves returning both data and an error.
  • While applicable to all subscriptions, only those requiring EOF handling would make use of it.

3. Introduce a Separate Subscription Type for EOF Handling

Creating a specialized subscription type that supports EOF handling.**

Advantages:

  • Restricts code changes to areas where they are needed.
  • The EOF-aware subscription is used only where applicable, leaving existing code unchanged.

After some discussion, we decided to proceed with option 3.

Additional Refactoring Considerations:
The subscription package would benefit from some refactoring. Specifically, the logic currently residing in the subscription.Next() function should be moved to a Streamer object. The subscription type should focus solely on reading from and writing to itself, while the Streamer should be responsible for determining whether to write in an ordered manner. This refactoring should be included in option 3 to help simplify the overall package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preserve Stale Bot repellent S-Access
Projects
None yet
Development

No branches or pull requests

4 participants