luoyuxia commented on code in PR #3864:
URL: https://github.com/apache/fluss/pull/3864#discussion_r3719048100
##########
fluss-server/src/main/java/org/apache/fluss/server/replica/HistoricalLakeLookupManager.java:
##########
@@ -259,39 +312,71 @@ void invalidateTableLookuper(long tableId) {
lakeTableLookupers.invalidate(tableId);
}
+ int cachedTableCount() {
+ return lakeTableLookupers.asMap().size();
+ }
+
+ Counter capacityEvictions() {
+ return capacityEvictions;
+ }
+
+ int numInflightRequests() {
+ return maxQueuedHistoricalRequests - lookupPermits.availablePermits();
+ }
+
+ void reconfigure(Configuration newConf) {
+ checkNotNull(newConf, "newConf must not be null.");
+ boolean lakeConfigChanged;
+ synchronized (this) {
Review Comment:
Thanks. The publication order is intentional rather than requiring an atomic
pair. Both fields are volatile: reconfigure publishes `conf` before
`lakeConfigVersion`, while lookup reads the version before `conf`. Therefore,
if a lookup observes the new version, the volatile happens-before relationship
guarantees that it also observes the corresponding configuration.
The only possible mixed observation is old version + new configuration. That
is safe: an existing old lookuper may finish the lookup as a pre-update
request, while a newly created lookuper uses the new configuration but carries
the old version and is retired by `invalidateAll()` or by the next lookup
through the version mismatch. The unsafe new version + old configuration
combination cannot occur, so an atomic holder is not required here.
##########
fluss-server/src/main/java/org/apache/fluss/server/replica/HistoricalLakeLookupManager.java:
##########
@@ -259,39 +312,71 @@ void invalidateTableLookuper(long tableId) {
lakeTableLookupers.invalidate(tableId);
}
+ int cachedTableCount() {
+ return lakeTableLookupers.asMap().size();
+ }
+
+ Counter capacityEvictions() {
+ return capacityEvictions;
+ }
+
+ int numInflightRequests() {
+ return maxQueuedHistoricalRequests - lookupPermits.availablePermits();
+ }
+
+ void reconfigure(Configuration newConf) {
+ checkNotNull(newConf, "newConf must not be null.");
+ boolean lakeConfigChanged;
+ synchronized (this) {
+ long newMaxBytes =
+ newConf.get(
+ ConfigOptions
+
.SERVER_HISTORICAL_PARTITION_LOOKUP_CACHE_MAX_DISK_SIZE)
+ .getBytes();
+ if (newMaxBytes != budgetManager.maxBytes()) {
+ budgetManager.updateGlobalLimit(newMaxBytes);
+ }
+
+ lakeConfigChanged = hasLakeConfigChanged(conf, newConf);
+ // Publish the configuration before its version. A lookup that
observes the new version
+ // must also observe the matching configuration snapshot.
+ conf = newConf;
+ if (lakeConfigChanged) {
+ lakeConfigVersion++;
+ }
+ }
Review Comment:
Thanks. The publication order is intentional rather than requiring an atomic
pair. Both fields are volatile: reconfigure publishes `conf` before
`lakeConfigVersion`, while lookup reads the version before `conf`. Therefore,
if a lookup observes the new version, the volatile happens-before relationship
guarantees that it also observes the corresponding configuration.
The only possible mixed observation is old version + new configuration. That
is safe: an existing old lookuper may finish the lookup as a pre-update
request, while a newly created lookuper uses the new configuration but carries
the old version and is retired by `invalidateAll()` or by the next lookup
through the version mismatch. The unsafe new version + old configuration
combination cannot occur, so an atomic holder is not required here.
--
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]
