[ 
https://issues.apache.org/jira/browse/HADOOP-19946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18141848#comment-18141848
 ] 

ASF GitHub Bot commented on HADOOP-19946:
-----------------------------------------

anmolanmol1234 commented on code in PR #8830:
URL: https://github.com/apache/hadoop/pull/8830#discussion_r4522633573


##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/BlobLayoutCache.java:
##########
@@ -0,0 +1,610 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.azurebfs.services;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.Expiry;
+import com.github.benmanes.caffeine.cache.Ticker;
+import org.checkerframework.checker.nullness.qual.NonNull;
+
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.azurebfs.contracts.services.BlobLayoutResponse;
+import 
org.apache.hadoop.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder;
+
+import static org.apache.hadoop.fs.azurebfs.services.AbfsInputStream.LOG;
+
+/**
+ * BlobLayoutCache is a singleton class that manages caching of BlobLayout 
objects for Azure Blob File System (ABFS).
+ * It uses a Caffeine cache to store layout information for blobs, optimizing 
range fetches and minimizing redundant network calls.
+ *
+ * <p>Key Features:</p>
+ * <ul>
+ *   <li>Evicts cache entries based on idle timeout and cache weight (number 
of cached ranges).</li>
+ *   <li>Tracks active streams to prevent eviction while a stream is open.</li>
+ *   <li>Manages in-flight promises for range fetches to avoid duplicate 
requests.</li>
+ *   <li>Provides methods to register/deregister streams, retrieve cached 
layouts, compute gaps, and manage promises.</li>
+ * </ul>
+ *
+ * <p>Usage:</p>
+ * <ul>
+ *   <li>Call {@link #registerStream(String, long)} when a stream is 
opened.</li>
+ *   <li>Call {@link #deregisterStream(String)} when a stream is closed.</li>
+ *   <li>Use {@link #getBlobLayout(String, long, long)} and {@link 
#getGaps(String, long, long)} to query cached layout info.</li>
+ *   <li>Use {@link #putBlobLayout(String, BlobLayoutResponse, long)} to 
update the cache with new layout info.</li>
+ *   <li>Use {@link #getBridgeGap(String, long, long)} to determine the next 
range to fetch, considering in-flight and cached ranges.</li>
+ *   <li>Use {@link #removePromise(String, long, long)} to remove completed 
in-flight promises.</li>
+ * </ul>
+ */
+public final class BlobLayoutCache {
+
+  /**
+   * LayoutEntry represents a cache entry for a blob's layout.
+   * It tracks the BlobLayout, active stream count, and last deregistered time 
for eviction logic.
+   */
+  private static class LayoutEntry {
+
+    /** The BlobLayout instance for this entry. */
+    private final BlobLayout layout;
+
+    /** Indicates if the layout is present and valid. */
+    private AtomicBoolean isLayoutPresent = new AtomicBoolean(true);
+
+    /** Number of active streams using this layout. */
+    private final AtomicInteger activeStreams = new AtomicInteger(0);
+
+    /** Last time (in nanoseconds) when all streams were deregistered. Used 
for idle eviction. */
+    private final AtomicLong lastDeregisteredTimeInNanos = new AtomicLong(
+        Long.MAX_VALUE);
+
+    /**
+     * Constructs a LayoutEntry with the given content length.
+     * @param contentLength the length of the blob content
+     */
+    LayoutEntry(long contentLength) {
+      this.layout = new BlobLayout(contentLength);
+    }
+
+    /**
+     * Checks if the layout is unavailable (not present).
+     * @return true if layout is unavailable, false otherwise
+     */
+    boolean isLayoutUnavailable() {
+      return !isLayoutPresent.get();
+    }
+  }
+
+  /**
+   * Caffeine Cache instance replacing ConcurrentHashMap.
+   * Uses weighted eviction based on the number of ranges stored in each 
layout.
+   */
+  private final Cache<String, LayoutEntry> cache;
+
+  /** Idle timeout in milliseconds for cache eviction. */
+  private final long idleTimeoutMs;
+
+  /** Maximum cache weight (total number of cached ranges) before eviction. */
+  private final long maxCacheWeight;
+
+  /** Time source used for idle-eviction bookkeeping (injectable for tests). */
+  private final Ticker ticker;
+
+  /** Singleton instance of BlobLayoutCache. */
+  private static volatile BlobLayoutCache instance = null;
+
+  /** Guards against repeatedly logging the singleton config-mismatch warning. 
*/
+  private static volatile boolean configMismatchWarned = false;
+
+  /**
+   * Registry of in-flight promises for range fetches, keyed by file ETag.
+   * Each value is a thread-safe list of InFlightPromise objects.
+   */
+  private final ConcurrentHashMap<String, 
CopyOnWriteArrayList<InFlightPromise>>
+      promiseRegistry = new ConcurrentHashMap<>();
+
+  /**
+   * InFlightPromise represents a pending fetch for a specific range.
+   * @param start the start offset of the range
+   * @param end the end offset of the range
+   * @param future the CompletableFuture representing the fetch operation
+   */
+  public record InFlightPromise(long start, long end,
+      CompletableFuture<Void> future) {}
+
+  /**
+   * Executor to asynchronously execute get blob layout calls
+   */
+  private static final ExecutorService FETCH_EXECUTOR = new ThreadPoolExecutor(
+      8, 32, 60L, TimeUnit.SECONDS,
+      new LinkedBlockingQueue<>(1024), // Bounded queue to prevent OOM
+      new ThreadFactoryBuilder()
+          .setNameFormat("abfs-blob-layout-fetch-%d")
+          .setDaemon(true)
+          .build(),
+      // AbortPolicy keeps the executor non-blocking: when the pool and queue 
are
+      // saturated, submission throws RejectedExecutionException instead of 
running
+      // the fetch on the caller (read) thread. The submitter is responsible 
for
+      // cleaning up the in-flight promise so a later read can retry the gap.
+      new ThreadPoolExecutor.AbortPolicy());
+
+  /**
+   * Optional override for the fetch executor, used only by tests to inject a
+   * deterministic executor (e.g. one that always rejects). {@code null} means 
the
+   * shared {@link #FETCH_EXECUTOR} is used.
+   */
+  private static volatile ExecutorService fetchExecutorOverride = null;
+
+  static {
+    // Add fetch executor to JVM shutdown hook to ensure orderly shutdown of 
thread during JVM shutdown.
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      FETCH_EXECUTOR.shutdown();
+      try {
+        if (!FETCH_EXECUTOR.awaitTermination(5, TimeUnit.SECONDS)) {
+          FETCH_EXECUTOR.shutdownNow();
+        }
+      } catch (InterruptedException e) {
+        FETCH_EXECUTOR.shutdownNow();
+      }
+    }));
+  }
+
+  /**
+   * Private constructor for singleton pattern.
+   * @param evictionTime the idle timeout in minutes for cache eviction
+   * @param maxCacheWeight the maximum total number of cached ranges before 
eviction
+   * @param ticker the time source used for idle-eviction bookkeeping
+   */
+  private BlobLayoutCache(long evictionTime, long maxCacheWeight, Ticker 
ticker) {
+    this.idleTimeoutMs = TimeUnit.MINUTES.toMillis(evictionTime);
+    this.maxCacheWeight = maxCacheWeight;
+    this.ticker = ticker;
+    this.cache = Caffeine.newBuilder()
+        .maximumWeight(maxCacheWeight)
+        .ticker(ticker)
+        // Weight is determined by the number of cached ranges in the layout
+        .weigher(
+            (String key, LayoutEntry entry) -> entry.layout.getRangeMapSize())
+        .expireAfter(new Expiry<String, LayoutEntry>() {
+          /**
+           * Calculates the expiry time for a cache entry based on active 
streams and idle time.
+           * @param value the LayoutEntry
+           * @param currentTime the current time in nanoseconds
+           * @return the time in nanoseconds until expiry
+           */
+          private long calculateExpiry(LayoutEntry value, long currentTime) {
+            long lastDeregistered = value.lastDeregisteredTimeInNanos.get();
+            // Do not expire while a stream is active, or before the first
+            // deregistration has happened (sentinel Long.MAX_VALUE). The 
latter
+            // check also avoids long overflow in the subtraction below.
+            if (value.activeStreams.get() > 0
+                || lastDeregistered == Long.MAX_VALUE) {
+              return Long.MAX_VALUE;
+            }
+
+            // How long has it been since the last stream closed?
+            long elapsedNanos = currentTime - lastDeregistered;
+            long thresholdNanos = TimeUnit.MILLISECONDS.toNanos(
+                idleTimeoutMs);
+
+            // If we are already past the threshold, expire immediately 
(return 0)
+            // Otherwise, return the remaining time until the threshold is hit
+            return Math.max(0, thresholdNanos - elapsedNanos);
+          }
+
+          @Override
+          public long expireAfterCreate(@NonNull String key,
+              @NonNull LayoutEntry value, long currentTime) {
+            return calculateExpiry(value, currentTime);
+          }
+
+          @Override
+          public long expireAfterUpdate(@NonNull String key,
+              @NonNull LayoutEntry value, long currentTime,
+              long currentDuration) {
+            return calculateExpiry(value, currentTime);
+          }
+
+          @Override
+          public long expireAfterRead(@NonNull String key,
+              @NonNull LayoutEntry value, long currentTime,
+              long currentDuration) {
+            // LEASE LOGIC: If streams are active, the entry never expires.
+            // Once activeStreams == 0, the 5-minute idle clock starts.
+            return calculateExpiry(value, currentTime);
+          }
+        })
+        .removalListener((key, value, cause) -> {
+          // Cleanup corresponding promises when an entry is evicted from cache
+          if (key != null) {
+            promiseRegistry.remove(key);
+          }
+        })
+        .build();
+  }
+
+  /**
+   * Returns the process-wide singleton instance of BlobLayoutCache, creating 
it
+   * if necessary.
+   * <p>
+   * The cache is intentionally shared across all filesystem instances in the 
JVM
+   * so that its total memory footprint stays bounded by a single weight 
budget.
+   * As a consequence, the {@code evictionTime} and {@code maxCacheWeight} 
supplied
+   * by the <em>first</em> caller win; values from later callers are ignored. 
When a
+   * later caller requests a different configuration, a one-time warning is 
logged
+   * rather than silently discarding it.
+   * </p>
+   * @param evictionTime the idle timeout in minutes for cache eviction
+   * @param maxCacheWeight the maximum total number of cached ranges before 
eviction
+   * @return the singleton BlobLayoutCache instance
+   */
+  public static BlobLayoutCache getInstance(long evictionTime,
+      long maxCacheWeight) {
+    BlobLayoutCache local = instance;
+    if (local == null) {
+      synchronized (BlobLayoutCache.class) {
+        local = instance;
+        if (local == null) {
+          local = new BlobLayoutCache(evictionTime, maxCacheWeight,
+              Ticker.systemTicker());
+          instance = local;
+        }
+      }
+    }
+    warnIfConfigDiffers(local, evictionTime, maxCacheWeight);
+    return local;
+  }
+
+  /**
+   * Logs a one-time warning if a caller requests a configuration that differs
+   * from the one the singleton was initialized with.
+   */
+  private static void warnIfConfigDiffers(BlobLayoutCache existing,
+      long evictionTime, long maxCacheWeight) {
+    if (configMismatchWarned) {
+      return;
+    }
+    long requestedIdleMs = TimeUnit.MINUTES.toMillis(evictionTime);
+    if (existing.idleTimeoutMs != requestedIdleMs
+        || existing.maxCacheWeight != maxCacheWeight) {
+      configMismatchWarned = true;
+      LOG.warn("BlobLayoutCache is process-wide and already initialized with "
+              + "evictionMs={}, maxWeight={}; ignoring requested 
evictionMs={}, "
+              + "maxWeight={}.", existing.idleTimeoutMs, 
existing.maxCacheWeight,
+          requestedIdleMs, maxCacheWeight);
+    }
+  }
+
+  /**
+   * Creates an isolated (non-singleton) cache instance for deterministic 
testing,
+   * allowing a custom {@link Ticker} so idle-based eviction can be exercised
+   * without real time passing.
+   * @param evictionTime the idle timeout in minutes for cache eviction
+   * @param maxCacheWeight the maximum total number of cached ranges before 
eviction
+   * @param ticker the time source to use
+   * @return a fresh, isolated BlobLayoutCache instance
+   */
+  @VisibleForTesting
+  static BlobLayoutCache newInstanceForTesting(long evictionTime,
+      long maxCacheWeight, Ticker ticker) {
+    return new BlobLayoutCache(evictionTime, maxCacheWeight, ticker);
+  }
+
+  /**
+   * Forces any pending cache maintenance (eviction/expiration) to run. For 
tests.
+   */
+  @VisibleForTesting
+  void cleanUp() {
+    cache.cleanUp();
+  }
+
+  /**
+   * Returns true if an entry for the given key is currently present (after 
running
+   * pending maintenance). For tests.
+   * @param key the cache key (file ETag)
+   * @return true if the entry is still cached
+   */
+  @VisibleForTesting
+  boolean isCached(String key) {
+    cache.cleanUp();
+    return cache.getIfPresent(key) != null;
+  }
+
+  /**
+   * Returns the approximate number of entries in the cache (after running 
pending
+   * maintenance). For tests.
+   * @return the estimated cache size
+   */
+  @VisibleForTesting
+  long estimatedSize() {
+    cache.cleanUp();
+    return cache.estimatedSize();
+  }
+
+  /**
+   * Returns the shared executor used to run asynchronous blob layout fetch 
tasks.
+   * Package-private: intended for use by {@code AbfsInputStream} only.
+   * @return the shared fetch executor
+   */
+  static ExecutorService getFetchExecutor() {
+    ExecutorService override = fetchExecutorOverride;
+    return override != null ? override : FETCH_EXECUTOR;
+  }
+
+  /**
+   * Overrides the fetch executor for tests so rejection/non-blocking 
behaviour can
+   * be exercised deterministically. Callers must invoke
+   * {@link #resetFetchExecutorForTesting()} afterwards.
+   * @param executor the executor to use, or {@code null} to reset
+   */
+  @VisibleForTesting
+  static void setFetchExecutorForTesting(ExecutorService executor) {
+    fetchExecutorOverride = executor;
+  }
+
+  /**
+   * Restores the default shared fetch executor after a test override. For 
tests.
+   */
+  @VisibleForTesting
+  static void resetFetchExecutorForTesting() {
+    fetchExecutorOverride = null;
+  }
+
+  /**
+   * Returns the number of in-flight fetch promises currently registered for 
the
+   * given file ETag. Used by tests to assert that promises are not leaked. 
For tests.
+   * @param fileETag the file ETag key
+   * @return the number of in-flight promises, or 0 if none are registered
+   */
+  @VisibleForTesting
+  int getInFlightPromiseCount(String fileETag) {
+    CopyOnWriteArrayList<InFlightPromise> promises = 
promiseRegistry.get(fileETag);
+    return promises == null ? 0 : promises.size();
+  }
+
+  /**
+   * Registers a stream for the given file ETag and content length.
+   * Increments the active stream count and blocks eviction.
+   * @param fileETag the file ETag
+   * @param contentLength the content length of the blob
+   */
+  public void registerStream(String fileETag, long contentLength) {
+    cache.asMap().compute(fileETag, (key, entry) -> {
+      if (entry == null) {
+        entry = new LayoutEntry(contentLength);
+      }
+      entry.activeStreams.incrementAndGet();
+      entry.lastDeregisteredTimeInNanos.set(
+          Long.MAX_VALUE); // Block eviction logic
+      return entry;
+    });
+  }
+
+  /**
+   * Deregisters a stream for the given file ETag.
+   * Decrements the active stream count and updates the last deregistered time 
for eviction.
+   * @param fileETag the file ETag
+   */
+  public void deregisterStream(String fileETag) {
+    cache.asMap().computeIfPresent(fileETag, (key, entry) -> {
+      // Clamp at zero so unbalanced deregister calls cannot drive the counter
+      // negative and keep resetting the idle clock.
+      int remaining = entry.activeStreams.updateAndGet(v -> v > 0 ? v - 1 : 0);
+      if (remaining == 0) {
+        entry.lastDeregisteredTimeInNanos.set(ticker.read());
+      }
+      return entry;
+    });
+  }
+
+  /**
+   * Retrieves the cached blob layout ranges for the specified key and range.
+   * @param key the cache key (file ETag)
+   * @param start the start offset
+   * @param end the end offset
+   * @return a list of BlobRange objects, or null if not available
+   */
+  public List<BlobLayout.BlobRange> getBlobLayout(final String key,
+      final long start, final long end) {
+    LayoutEntry layoutEntry = cache.getIfPresent(key);
+    return (layoutEntry == null || layoutEntry.isLayoutUnavailable())
+        ? null : layoutEntry.layout.getRanges(start, end);
+  }
+
+  /**
+   * Retrieves the gaps (unfetched ranges) in the cached layout for the 
specified key and range.
+   * @param key the cache key (file ETag)
+   * @param start the start offset
+   * @param end the end offset
+   * @return a list of BlobRange objects representing gaps, or null if not 
available
+   */
+  public List<BlobLayout.BlobRange> getGaps(final String key,
+      final long start, final long end) {
+    LayoutEntry layoutEntry = cache.getIfPresent(key);
+    return layoutEntry == null || layoutEntry.isLayoutUnavailable()
+        ? null : layoutEntry.layout.getGaps(start, end);
+  }
+
+  /**
+   * Determines the next range to fetch (bridge gap) for the given file ETag, 
position, and max fetch size.
+   * Considers both cached and in-flight (promised) ranges to avoid redundant 
fetches.
+   * @param fileETag the file ETag
+   * @param pos the current position
+   * @param maxFetch the maximum fetch size
+   * @return a BlobRange representing the next gap to fetch, or null if none
+   */
+  public BlobLayout.BlobRange getBridgeGap(String fileETag,
+      long pos,
+      long maxFetch) {
+    LayoutEntry layoutEntry = cache.getIfPresent(fileETag);
+    if (layoutEntry == null || layoutEntry.isLayoutUnavailable()) {
+      return null;
+    }
+
+    long fetchStart = layoutEntry.layout.getFetchStart(pos, maxFetch);
+    if (fetchStart == -1) {
+      return null;
+    }
+
+    long nextPromisedStart = Long.MAX_VALUE;
+    CopyOnWriteArrayList<InFlightPromise> filePromises = promiseRegistry.get(
+        fileETag);
+    if (filePromises != null) {
+      for (InFlightPromise p : filePromises) {
+        if (p.start() > pos && p.start() < nextPromisedStart) {
+          nextPromisedStart = p.start();
+        }
+      }
+    }
+
+    long nextCachedStart = layoutEntry.layout.getNextCachedStart(pos);
+    long nextWall = Math.min(nextCachedStart, nextPromisedStart);
+
+    long fetchEnd;
+    if (nextWall != Long.MAX_VALUE && (nextWall - fetchStart <= maxFetch)) {
+      fetchEnd = nextWall - 1;
+    } else {
+      fetchEnd = Math.min(layoutEntry.layout.getContentLength() - 1,
+          fetchStart + maxFetch - 1);
+    }
+
+    return new BlobLayout.BlobRange(fetchStart, fetchEnd, null);
+  }
+
+  /**
+   * Updates the cache with a new BlobLayoutResponse for the specified key.
+   * <p>
+   * Semantics:
+   * <ul>
+   *   <li>A {@code null} response represents a <em>transient</em> fetch 
failure. The
+   *   entry is left untouched (still available) so the next read can retry 
the gap;
+   *   it is <strong>not</strong> permanently poisoned.</li>
+   *   <li>A non-null response with no endpoints and no ranges means the 
server has
+   *   no distributed layout for this blob. The entry is marked unavailable so 
callers
+   *   fall back to host-URL based fetches.</li>
+   *   <li>A non-null response with layout data is merged in and the entry is 
(re)marked
+   *   available, allowing recovery from a previous unavailable state.</li>
+   * </ul>
+   * The update runs inside {@link 
java.util.concurrent.ConcurrentHashMap#compute}
+   * (via the cache's map view) so that Caffeine re-evaluates the entry weight 
after
+   * ranges are added; otherwise weight-based eviction would never see the 
growth.
+   * </p>
+   * @param key the cache key (file ETag)
+   * @param layoutResponse the BlobLayoutResponse containing new layout info
+   * @param contentLength the content length of the blob
+   */
+  public void putBlobLayout(final String key,
+      final BlobLayoutResponse layoutResponse,
+      final long contentLength) {
+    cache.asMap().compute(key, (k, existing) -> {
+      LayoutEntry entry = (existing != null)

Review Comment:
   A new LayoutEntry starts with lastDeregisteredTimeInNanos = Long.MAX\_VALUE, 
and calculateExpiry treats that as "never expire". Normally registerStream and 
deregisterStream manage this value.
   
   If a fetch finishes after the entry was evicted and all streams were closed 
(the case in testPutRecreatesEntryAfterEviction), putBlobLayout recreates the 
entry with activeStreams = 0. No deregisterStream call will ever run for it, so 
it never expires by time. Only weight\-based eviction can remove it.
   
   Can we set lastDeregisteredTimeInNanos = ticker.read() when putBlobLayout 
creates an entry that has no active streams? It would also help to extend that 
test to assert the recreated entry eventually expires.





> ABFS: Enable data locality aware reads for Azure Blob Storage
> -------------------------------------------------------------
>
>                 Key: HADOOP-19946
>                 URL: https://issues.apache.org/jira/browse/HADOOP-19946
>             Project: Hadoop Common
>          Issue Type: New Feature
>          Components: fs/azure
>            Reporter: Manish Bhatt
>            Assignee: Manish Bhatt
>            Priority: Major
>              Labels: pull-request-available
>
> Introduce data locality support in ABFS to improve read performance for Azure 
> Blob Storage by routing read requests to the endpoint that hosts the required 
> data range. The goal is to reduce latency, improve throughput, and make read 
> operations more efficient for large files and distributed workloads. This 
> work adds blob layout discovery, range-to-endpoint mapping, locality-aware 
> range selection, and caching of layout metadata to avoid repeated lookups. It 
> also enables segmented read-ahead aligned with locality boundaries and adds 
> safeguards for concurrency, eviction, timeout handling, and failure 
> propagation needed for reliable production-scale reads.



--
This message was sent by Atlassian Jira
(v10.3.26#10030026)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to