fix: don't fail on startup if disk usage monitor is disabled #10749
Conversation
Allows us to swap out the disk usage monitor with alternative implementations.
Some components try to register themselves as disk usage listener on startup. When disk usage monitoring was disabled, these components would fail with an NPE during startup. Instead of checking for null everywhere the monitor is used, we register a no-op disk usage monitor that other components can use for registering but that won't deliver any notifications.
Test Results 861 files - 78 861 suites - 78 1h 4m 13s ⏱️ - 40m 51s For more details on these failures, see this check. Results for commit aafcdef. ± Comparison against base commit ff72a85. ♻️ This comment has been updated with latest results. |
|
I tested manually and considered adding a test but writing a full integration test for this specific case felt a bit heavy-handed. Happy to be convinced otherwise though :) |
| @@ -23,14 +27,43 @@ void startupInternal( | |||
| final ActorFuture<BrokerStartupContext> startupFuture) { | |||
|
|
|||
| final var data = brokerStartupContext.getBrokerConfiguration().getData(); | |||
|
|
|||
| if (!data.isDiskUsageMonitoringEnabled()) { | |||
| brokerStartupContext.setDiskSpaceUsageMonitor(new DisabledDiskUsageMonitor()); | |||
| final ActorFuture<BrokerStartupContext> shutdownFuture) { | ||
|
|
||
| final var diskSpaceUsageMonitor = brokerShutdownContext.getDiskSpaceUsageMonitor(); | ||
| if (diskSpaceUsageMonitor instanceof DiskSpaceUsageMonitorActor actor) { |
There was a problem hiding this comment.
🔧 It would be better to add a close method to the DiskSpaceUsageMonitorInterface interface.
There was a problem hiding this comment.
Yeah, good point 👍 I've just added an extends AsyncClosable to the DiskUsageMonitor interface to implement this.
This simplifies the logic around stopping the monitor and prevents that potential new implementations would not be closed if we would forget to add them as a special case to the startup step.
|
bors r+ |
|
Build succeeded: |
zeebe-bors-camunda
bot
deleted the
10735-optional-disk-usage-monitoring
branch
|
Backport failed for Please cherry-pick the changes locally. git fetch origin stable/8.0
git worktree add -d .worktree/backport-10749-to-stable/8.0 origin/stable/8.0
cd .worktree/backport-10749-to-stable/8.0
git checkout -b backport-10749-to-stable/8.0
ancref=$(git merge-base ff72a85d563b4c4dfa272e168c57d4bbaefe0964 aafcdef8a567e01810b94321c19eaec6a3b7163c)
git cherry-pick -x $ancref..aafcdef8a567e01810b94321c19eaec6a3b7163c |
|
Successfully created backport PR #10751 for |
korthout
added
the
version:8.1.3
Some components try to register themselves as disk usage listener on startup. When disk usage monitoring was disabled, these components would fail with an NPE during startup.
Instead of checking for null everywhere the monitor is used, we register a no-op disk usage monitor that other components can use for registering but that won't deliver any notifications.
closes #10735