Skip to content

Health Check Improvements #254

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

Merged
merged 25 commits into from
May 8, 2025
Merged

Health Check Improvements #254

merged 25 commits into from
May 8, 2025

Conversation

stevensJourney
Copy link
Collaborator

@stevensJourney stevensJourney commented May 5, 2025

Overview

The PowerSync service uses a probing system to communicate health check statuses. Internally, the probe state is updated when the service becomes ready or active. This state is maintained in memory and can be exposed externally via the filesystem or HTTP endpoints.

Previously, probe state was always exposed via the filesystem by writing timestamps to .probes/startup, .probes/ready, and .probes/poll. This required the service to run with write access to the filesystem, which isn't always feasible or recommended. There was no way to disable filesystem probes.

In addition, the same probe state was exposed via HTTP routes, but only if the RouterEngine started an HTTP server. This occurred only in API and UNIFIED modes. In SYNC mode, the only way to access probe state externally was through the filesystem.

What's new

This PR introduces configurable probe types through the PowerSync configuration file. You can now enable or disable probe outputs via YAML:

  healthcheck:
    probes:
      use_filesystem: true
      use_http: true

The specified probe_types determine how probe state is exposed. Selecting http will now start an HTTP server even when running in SYNC mode.

Backwards compatibility
If no healthchecks configuration is provided, the service retains its previous behavior.

Implementation

Internally this moves some configuration from the service runner to a new shared module. This should remove some minor duplication throughout our hosted codebases.

Copy link

changeset-bot bot commented May 5, 2025

🦋 Changeset detected

Latest commit: 6791d8b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@powersync/service-core Minor
@powersync/service-image Minor
@powersync/service-types Minor
@powersync/lib-services-framework Minor
@powersync/service-module-core Minor
@powersync/service-core-tests Patch
@powersync/service-module-mongodb-storage Patch
@powersync/service-module-mongodb Patch
@powersync/service-module-mysql Patch
@powersync/service-module-postgres-storage Patch
@powersync/service-module-postgres Patch
test-client Patch
@powersync/service-schema Minor
@powersync/lib-service-postgres Patch
@powersync/service-rsocket-router Patch
@powersync/lib-service-mongodb Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@stevensJourney stevensJourney requested a review from Copilot May 8, 2025 08:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the PowerSync service’s health check mechanism by making probe exposure configurable via YAML and migrating related configuration from service runners to a shared module. Key changes include:

  • Passing explicit service modes and configurations to ServiceContextContainer in connection, migration, and compact actions.
  • Adding configurable health check probe options to support both HTTP and filesystem implementations.
  • Refactoring core module initialization to include router and metric configurations with the new probe mechanism.

Reviewed Changes

Copilot reviewed 48 out of 48 changed files in this pull request and generated no comments.

File Description
packages/service-core/src/entry/commands/* Updated ServiceContextContainer instantiation to include explicit mode and configuration.
modules/module-core/* Moved and refactored router, health check, and metrics configuration to a shared module.
libs/lib-services/src/container.ts Switched default health check probe implementation from filesystem to in-memory.
Various .changeset/* files Updated versioning and release notes for the new probe configuration and API changes.
Comments suppressed due to low confidence (2)

modules/module-core/src/CoreModule.ts:111

  • Destructuring healthcheck.probes assumes that 'healthcheck' is always defined. To maintain backwards compatibility when no healthcheck configuration is supplied, provide a default value (e.g. using a default object) to avoid runtime errors.
const { configuration: { healthcheck: { probes } } } = context;

modules/module-core/src/CoreModule.ts:129

  • The result of setInterval is not assigned to the 'timer' variable, which means clearInterval in the stop lifecycle will never clear the timer. Assign the interval to 'timer' so that it can be properly cleared during shutdown.
setInterval(() => { ... }, 5_000);

@stevensJourney stevensJourney marked this pull request as ready for review May 8, 2025 13:15
@stevensJourney stevensJourney requested a review from rkistner May 8, 2025 13:15
rkistner
rkistner previously approved these changes May 8, 2025
Copy link
Contributor

@rkistner rkistner left a comment

Choose a reason for hiding this comment

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

I like the new CoreModule structure here.

One thing that isn't clear from the docs - if you specify use_http and run with -r sync, would that start an http server just for the probes?

@stevensJourney
Copy link
Collaborator Author

I like the new CoreModule structure here.

One thing that isn't clear from the docs - if you specify use_http and run with -r sync, would that start an http server just for the probes?

Yes, starting in that option will register the probe routes on the RouterEngine. The router engine will then start an HTTP server. Only probe routes will be available on this router/server.

@rkistner
Copy link
Contributor

rkistner commented May 8, 2025

One request - can we make this warning more friendly:

warn: No routes have been registered. Router Engine will not continue.

I expect that running with -r sync without http probes would be common/expected usage, so shouldn't generate a warning.

@stevensJourney
Copy link
Collaborator Author

One request - can we make this warning more friendly:

warn: No routes have been registered. Router Engine will not continue.

I expect that running with -r sync without http probes would be common/expected usage, so shouldn't generate a warning.

I've updated this to an info level log which logs Router Engine will not start an HTTP server as no routes have been registered.

@stevensJourney stevensJourney merged commit ca0a566 into main May 8, 2025
21 checks passed
@stevensJourney stevensJourney deleted the probes branch May 8, 2025 14:35
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.

2 participants