pradeeee opened a new pull request, #18608:
URL: https://github.com/apache/pinot/pull/18608
## Problem
The `PredownloadScheduler` runs in a separate **predownload container** that
starts before the main Pinot server process. Its purpose is to pre-download
segments from deep storage so the server can start faster.
Previously, the `initializeMetricsReporter()` method in
`PredownloadScheduler` only created `PredownloadMetrics` but **never
initialized the `ServerMetrics` registry**. As a result:
1. `PredownloadMetrics` internally depends on `ServerMetrics.get()` for
reporting server-level meters and timers.
2. Since `ServerMetrics` was never initialized in the predownload container
lifecycle, `ServerMetrics.get()` always returned the **NOOP instance**.
3. This meant **no server-level metrics (download latency, failure counts,
etc.) were emitted** from the predownload container — metrics were silently
dropped.
### Call chain showing the gap
```
PredownloadScheduler.start()
→ initializeMetricsReporter()
→ new PredownloadMetrics() // created
→ ServerMetrics was never set up // missing — falls back to NOOP
```
In contrast, the normal server startup path initializes `ServerMetrics` via
`PinotMetricUtils.getPinotMetricsRegistry()` before any metrics are used.
## Changes
This PR adds `ServerMetrics` initialization to
`PredownloadScheduler.initializeMetricsReporter()`:
1. Creates a `ServerConf` from the existing `_pinotConfig`
2. Initializes `PinotMetricsRegistry` via
`PinotMetricUtils.getPinotMetricsRegistry()`
3. Creates and registers a `ServerMetrics` instance with the proper prefix,
table-level metrics config, and allowed tables
### Graceful fallback
The initialization is wrapped in a `try-catch(Throwable)` so that if the
configured `PinotMetricsFactory` cannot be created (e.g., because a
vendor-specific metrics client is not yet available during the predownload
lifecycle), the predownload process **continues with the default NOOP
`ServerMetrics`** rather than crashing. This ensures the predownload container
remains resilient regardless of the metrics backend availability.
### Before (no ServerMetrics)
```java
void initializeMetricsReporter() {
_predownloadMetrics = new PredownloadMetrics();
PredownloadStatusRecorder.registerMetrics(_predownloadMetrics);
}
```
### After (ServerMetrics initialized with fallback)
```java
void initializeMetricsReporter() {
try {
ServerConf serverConf = new ServerConf(_pinotConfig);
PinotMetricsRegistry metricsRegistry =
PinotMetricUtils.getPinotMetricsRegistry(serverConf.getMetricsConfig());
ServerMetrics serverMetrics = new ServerMetrics(...);
serverMetrics.initializeGlobalMeters();
ServerMetrics.register(serverMetrics);
} catch (Throwable t) {
LOGGER.error("Failed to initialize ServerMetrics; "
+ "continuing with NOOP instance", t);
}
_predownloadMetrics = new PredownloadMetrics();
PredownloadStatusRecorder.registerMetrics(_predownloadMetrics);
}
```
## Test Plan
- Verified that the predownload container starts successfully with and
without a metrics factory on the classpath
- When the metrics factory is available, `ServerMetrics` is properly
initialized and metrics are emitted
- When the metrics factory is unavailable, the catch block logs the error
and the container continues with NOOP metrics without crashing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
