Skip to content

Conversation

@harvinderRathore
Copy link
Contributor

@harvinderRathore harvinderRathore commented Sep 21, 2025

Problem
On broker shutdown, active streaming subscriptions previously completed with OK,
which prevents clients from detecting outage and retrying properly.

Fix
Wrap subscribe streams with a tokio::select! that also listens to the broker's
shutdown broadcast. When triggered, terminate the stream with
Status::unavailable("Databroker shutting down").

Testing

  • Manual: started broker (--insecure), subscribed via CLI, published values,
    sent SIGINT to broker. CLI shows “Server gone, Subscription stopped”.
  • Build/tests: cargo build, cargo test pass locally.

Linked issue
Fixes #125

Signed-off-by: Harvinder Singh Rathor [email protected]

@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.08%. Comparing base (9b4c5be) to head (af7f1b0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
databroker/src/grpc/kuksa_val_v1/val.rs 0.00% 5 Missing ⚠️
databroker/src/grpc/sdv_databroker_v1/broker.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #171      +/-   ##
==========================================
- Coverage   62.10%   62.08%   -0.03%     
==========================================
  Files          37       37              
  Lines       17088    17094       +6     
==========================================
  Hits        10612    10612              
- Misses       6476     6482       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SebastianSchildt
Copy link
Contributor

Thank you for the contribution.

There are some minor issues with linitng (we are using cargo fmt and clippy). The pre-commit hoo can help with that, see the notes here https://github.com/eclipse-kuksa/kuksa-databroker/blob/main/CONTRIBUTING.md#pre-commit-set-up

Also, we require that commits are signed-off and that you "signed" the Eclipse Contributor Agreement, whereas signing means

  • making a free Eclipse account https://accounts.eclipse.org/user/register?destination=/user/login
  • In your Eclipse Account link your gihub identiy
  • "sign" the ECA, which basically means setting a checkmark and typing your name in the Eclipse account. Not much legalese there, you basically only confirm you contribute to an Eclipse project you are abiding by the license (Apache in our case) and that the work you provided is your own.

In case you get stuck, just write.

So mich for the formal stuff, wrt to "tech" content @rafaeling do you think these change makes sense/does not break anything?

Previously, active subscriptions ended with OK on broker shutdown,
which is misleading for clients. Now, subscription streams listen
to the broker shutdown broadcast and terminate with
Status::unavailable("Databroker shutting down").

Fixes eclipse-kuksa#125

Signed-off-by: Harvinder Singh Rathor <[email protected]>
@harvinderRathore harvinderRathore force-pushed the fix-subscription-unavailable branch from 37bb25e to 6c53813 Compare September 23, 2025 17:44
@harvinderRathore
Copy link
Contributor Author

@SebastianSchildt & @rafaeling
This update applies rustfmt formatting in val.rs to resolve the cargo fmt pre-commit failure.
No functional changes were made.
Requesting review and workflow approval.

@harvinderRathore harvinderRathore force-pushed the fix-subscription-unavailable branch from cd40102 to d28bade Compare September 23, 2025 21:41
@harvinderRathore harvinderRathore force-pushed the fix-subscription-unavailable branch from d28bade to af7f1b0 Compare September 23, 2025 21:55
@harvinderRathore
Copy link
Contributor Author

harvinderRathore commented Sep 23, 2025

Screenshot from 2025-09-24 03-46-00

@SebastianSchildt & @rafaeling
CI is passed. Verified on my repo. Ref the image
No functional changes were made.
Requesting review and workflow approval.

@rafaeling rafaeling self-requested a review September 24, 2025 07:42
Copy link
Contributor

@rafaeling rafaeling left a comment

Choose a reason for hiding this comment

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

Looks good to me and @lukasmittag, the PR doesn't introduces any break changes to kuksa.val.v2.
However, UNAVAILABLE error code is used differently in kuksa.val.v2, we would have to re-check again in the future how to handle it correctly
Bug created: #173

@SebastianSchildt SebastianSchildt merged commit 67edbf4 into eclipse-kuksa:main Sep 24, 2025
36 checks passed
@harvinderRathore harvinderRathore deleted the fix-subscription-unavailable branch September 24, 2025 12:36
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.

Databroker shall send UNAVAILABLE if terminating subscriptions on shutdown

3 participants