FrankChen021 commented on code in PR #19739:
URL: https://github.com/apache/druid/pull/19739#discussion_r3652472127
##########
server/src/main/java/org/apache/druid/segment/loading/SegmentLoaderConfig.java:
##########
@@ -283,24 +286,40 @@ public SegmentLoaderConfig
setVirtualStoragePartialDownloadsEnabled(boolean enab
}
/**
- * Sets {@link #virtualStorage}.
+ * Sets {@link #virtualStorage}. When enabling it, settings that only apply
to classic on-disk historical caches are
+ * reset to values safe for on-demand virtual-storage caches, so that a
per-task config derived from a node's
+ * {@code druid.segmentCache} config (via {@link #withVirtualStorage}) does
not inherit historical-only behavior. This
+ * runs only for programmatic enabling; a config with {@code virtualStorage}
deserialized straight from JSON is left
+ * as the operator specified it.
*/
public SegmentLoaderConfig setVirtualStorage(boolean virtualStorage)
{
this.virtualStorage = virtualStorage;
+ if (virtualStorage) {
+ // On-demand virtual-storage caches never bootstrap or lazy-load at
startup, never warm the OS page cache, must
+ // not share a persistent info directory (each task cache has its own
ephemeral location), and must delete files
+ // as soon as all holds are released.
+ this.lazyLoadOnStart = false;
Review Comment:
[P2] Avoid sanitizing operator config through the Jackson setter
`setVirtualStorage` is also Jackson's bean mutator for the `virtualStorage`
property, so `JsonConfigProvider` calls this branch while loading
`druid.segmentCache`. Consequently operator config is not left untouched as
promised: for example, `virtualStorage=true` with `lazyLoadOnStart=true` can
have the latter applied first and then silently reset to false, changing
bootstrap behavior on existing virtual-storage Historicals. Direct JSON also
becomes property-order dependent. Keep this sanitization in
`withVirtualStorage(true)` or a dedicated per-task derivation method, leave the
bean setter as a simple assignment, and add a deserialization regression test.
--
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]
