Skip to content

Conversation

@jsdt
Copy link
Contributor

@jsdt jsdt commented Apr 3, 2025

Description of Changes

The energy monitor has no use, and was removed in private a while ago. This change uses the NullEnergyMonitor, so we don't do any work. Eventually we should purge all the energy monitor code.

Expected complexity level and risk

1

@jsdt jsdt requested a review from gefjon April 3, 2025 22:17
@jsdt jsdt enabled auto-merge April 3, 2025 23:31
Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

Easy enough. I'll make a tracking ticket to purge all the now-dead code.

@jsdt jsdt added this pull request to the merge queue Apr 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2025
@gefjon gefjon added this pull request to the merge queue Apr 4, 2025
Merged via the queue into master with commit 75098bc Apr 4, 2025
14 checks passed
kim added a commit that referenced this pull request Apr 16, 2025
PR #2550 removed performing the reducer call inside `spawn_blocking`,
thereby introducing the regression to never invoke the `on_panic`
callback.

This causes a database to continue accepting writes even if it is unable
to persist the transactions via its `Durability` impl. Instead, the
`on_panic` callback should remove the database from the
`HostController`, which renders _this_ database unavailable, but does
not affect any other databases managed by the controller.

The behavior is fine in `panic = abort` environments, yet until we have
a better way to propagate durability errors as values to the
`ModuleHost`, we can't abort the server in multi-tenant environments.

Thus, restore the original behavior (using `spawn` instead of the more
expensive `spawn_blocking`).
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