PageCache warmup on Server Restarts and Segment Refresh - #16033
PageCache warmup on Server Restarts and Segment Refresh#16033praveenc7 wants to merge 13 commits into
Conversation
d27abf6 to
0def567
Compare
ae48688 to
8110c5c
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #16033 +/- ##
============================================
+ Coverage 67.44% 67.65% +0.21%
- Complexity 1430 1450 +20
============================================
Files 3485 3498 +13
Lines 223874 225738 +1864
Branches 35300 35626 +326
============================================
+ Hits 150987 152733 +1746
- Misses 60890 60924 +34
- Partials 11997 12081 +84
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
# Conflicts: # pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerMeter.java # pinot-common/src/main/java/org/apache/pinot/common/utils/config/TableConfigSerDeUtils.java # pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java # pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java # pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java # pinot-server/src/main/java/org/apache/pinot/server/starter/ServerInstance.java # pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java # pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableConfig.java # pinot-spi/src/main/java/org/apache/pinot/spi/utils/builder/TableConfigBuilder.java
70006bc to
006052f
Compare
…tighter executors, simpler metrics
- Consolidate PageCacheWarmupConfig into a reusable nested Spec
{enabled, maxWarmupDurationSeconds, qpsLimit, policy} used as optional
onRestart/onRefresh blocks, giving per-case control without field duplication.
- Discover any live controller via HelixHelper.getControllerUrl(HelixManager)
(moved out of QueryWorkloadConfigUtils so QRI and warmup share one copy),
replacing the segment-download-URL parsing.
- Harden executors: fix TablesResource concurrent-warmup race (atomic claim),
replace prod asserts with guards, fix restart-loop early return, make the
per-query timeout effective, run controller HTTP fan-out on a dedicated
bounded pool (shut down on stop), cancel work on timeout.
- Simplify metrics to host-level only; fold timeout errors into server errors
and remove PAGE_CACHE_WARMUP_TIMEOUT_ERRORS.
- Fix TableConfigSerDeUtils.toZNRecord to serialize pageCacheWarmupConfig so it
survives ZK round-trip; add controller.page.cache.warmup.duration.ms config.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
|
||
| PinotFS pinotFS = PinotFSFactory.create(URIUtils.getUri(_pageCacheWarmupQueriesDataDir).getScheme()); | ||
| File tableDir = new File(_pageCacheWarmupQueriesDataDir, tableNameWithType); | ||
| File[] files = tableDir.listFiles(File::isFile); |
There was a problem hiding this comment.
Could we keep this lookup entirely within PinotFS? _pageCacheWarmupQueriesDataDir is resolved to a PinotFS above, and the upload/REST-read paths support filesystem URI schemes, but java.io.File.listFiles() can only enumerate local/POSIX paths. With an S3/HDFS-style data directory this will return null, causing onRefresh warmup to be silently skipped; queryFile.toURI() would also produce a file:// URI. This is especially relevant for multiple controllers, where the query files may need shared storage. PinotFS supports local disk through LocalPinotFS, so this could use pinotFS.listFiles(tableDirUri, false), filter entries with !pinotFS.isDirectory(uri), select by pinotFS.lastModified(uri), and then call pinotFS.open(uri).
There was a problem hiding this comment.
Addressed in fd08de7 and 0d12182. Query discovery now stays within PinotFS using listFilesWithMetadata(), filters directories through FileMetadata, selects the newest entry, and opens that same filesystem URI. It queries lastModified() only when an entry does not include a modification timestamp. Added coverage for S3/S3A raw keys, HDFS path-only metadata, LocalPinotFS, relative local paths, and missing directories.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The binary compact failure is expected because it checks for new delta in SPI class. This is harmless and this change is backward compatible the new field is defined as nullable |
Summary
Issue link : #15675
What this PR delivers
🔄 Typical flow
Create Query file
2. Server restart
Startup hook calls warm-up for each enabled table; queries run for the configured warmup duration
3. Segment refresh
Controller triggers warm-up for only the newly uploaded segments using the consistent protocol feature.
Backward compatibility
Fully opt-in – no existing table is affected until pageCacheWarmupConfig is set.
Query path remains unchanged; warm-up traffic stays on the secondary queue during refresh if BinaryWorkloadScheduler is enabled. During restart it uses the same primary queue since it is not competing with live traffic
Testing
unit test, manually tested using quick-starter and test actual servers