[
https://issues.apache.org/jira/browse/HADOOP-19946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18141905#comment-18141905
]
ASF GitHub Bot commented on HADOOP-19946:
-----------------------------------------
anmolanmol1234 commented on code in PR #8830:
URL: https://github.com/apache/hadoop/pull/8830#discussion_r4523806184
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/services/BlobLayoutCacheTest.java:
##########
@@ -0,0 +1,417 @@
+/**
+ * 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.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import org.apache.hadoop.fs.azurebfs.contracts.services.BlobLayoutResponse;
+
+import static
org.apache.hadoop.fs.azurebfs.constants.FileSystemConfigurations.DEFAULT_FS_AZURE_BLOB_LAYOUT_CACHE_MAX_COUNT;
+
+public class BlobLayoutCacheTest {
+
+ private BlobLayoutCache cache;
+
+ private final long contentLength = 100;
+
+ private BlobLayoutResponse layoutResponse;
+
+ @BeforeEach
+ public void setUp() {
+ cache = BlobLayoutCache.getInstance(1L,
+ DEFAULT_FS_AZURE_BLOB_LAYOUT_CACHE_MAX_COUNT);
+
+ this.layoutResponse = new BlobLayoutResponse();
+ layoutResponse.setRanges(List.of(new BlobLayoutResponse.Range(0, 9, 0),
+ new BlobLayoutResponse.Range(10, 19, 1),
+ new BlobLayoutResponse.Range(20, 29, 2),
+ new BlobLayoutResponse.Range(30, 39, 3)));
+ layoutResponse.setEndpoints(
+ Set.of(new BlobLayoutResponse.Endpoint(0, "host-a"),
+ new BlobLayoutResponse.Endpoint(1, "host-b"),
+ new BlobLayoutResponse.Endpoint(2, "host-c"),
+ new BlobLayoutResponse.Endpoint(3, "host-d")));
+ }
+
+ /**
+ * Tests registering and deregistering a stream in the BlobLayoutCache.
+ * <p>
+ * Verifies that after registering a stream, the blob layout is empty if no
elements are added.
+ * After deregistering, the blob layout remains empty, and no exceptions are
thrown.
+ */
+ @Test
+ public void testRegisterAndDeregisterStream() {
+ String eTag = new Throwable().getStackTrace()[0].getMethodName();
+ cache.registerStream(eTag, contentLength);
+ List<BlobLayout.BlobRange> blobRangeList = cache.getBlobLayout(eTag, 0,
+ 4);
+ Assertions.assertThat(blobRangeList)
+ .describedAs("List should be empty in case no element is added.")
+ .hasSize(0);
+ cache.deregisterStream(eTag);
+ blobRangeList = cache.getBlobLayout(eTag, 0, 4);
+ Assertions.assertThat(blobRangeList)
+ .describedAs(
+ "In case stream is deregistered, doesn't mean data is removed
immediately from the cache.")
+ .hasSize(0);
+ // No direct assertion, but should not throw
+ }
+
+ /**
+ * Tests putting and retrieving a blob layout in the BlobLayoutCache.
+ * <p>
+ * Verifies that after putting a blob layout, the correct ranges and hosts
are returned
+ * for a specified range.
+ */
+ @Test
+ public void testPutBlobLayoutAndGetBlobLayout() {
+ String eTag = new Throwable().getStackTrace()[0].getMethodName();
+ cache.registerStream(eTag, contentLength);
+ cache.putBlobLayout(eTag, layoutResponse, contentLength);
+
+ // If start = 0, and end = 10 (both inclusive), get blob layout will
return two entries
+ // 0-9 -> host-a
+ // 10-10 -> host-b
+ List<BlobLayout.BlobRange> blobRangeList = cache.getBlobLayout(eTag, 0,
10);
+ Assertions.assertThat(blobRangeList)
+ .describedAs("List should contains 2 elements.")
+ .hasSize(2);
+ Assertions.assertThat(blobRangeList.get(0).host())
+ .describedAs("First range should be host-a.")
+ .isEqualTo("host-a");
+ Assertions.assertThat(blobRangeList.get(1).host())
+ .describedAs("Second range should be host-b.")
+ .isEqualTo("host-b");
+ cache.deregisterStream(eTag);
+ }
+
+ /**
+ * Tests gap detection when content length is greater than the last byte
present in the layout.
+ * <p>
+ * Verifies that gaps are correctly identified and returned for the
specified range.
+ */
+ @Test
+ public void testGapWithContentLengthGreaterThanLastBytePresent() {
+ String eTag = new Throwable().getStackTrace()[0].getMethodName();
+ cache.registerStream(eTag, contentLength);
+ cache.putBlobLayout(eTag, layoutResponse, contentLength);
+
+ // If start = 35, and end = 45 (both inclusive), get Gaps will return one
entry
+ // 40-45 -> null
+ List<BlobLayout.BlobRange> gaps = cache.getGaps(eTag, 35, 45);
+ Assertions.assertThat(gaps)
+ .describedAs("List should contains 1 elements.")
+ .hasSize(1);
+ Assertions.assertThat(gaps.get(0).start())
+ .describedAs("Gap should start from 40.")
+ .isEqualTo(40);
+ Assertions.assertThat(gaps.get(0).end())
+ .describedAs("Gap should end at 45.")
+ .isEqualTo(45);
+ cache.deregisterStream(eTag);
+ }
+
+ /**
+ * Tests gap detection when content length is equal to the last byte present
in the layout.
+ * <p>
+ * Verifies that no gaps are returned, and the correct blob range and host
are returned for the specified range.
+ */
+ @Test
+ public void testGapWithContentLengthEqualToLastBytePresent() {
+ String eTag = new Throwable().getStackTrace()[0].getMethodName();
+ cache.registerStream(eTag, 40);
+ cache.putBlobLayout(eTag, layoutResponse, 40);
+
+ // If start = 35, and end = 45 (both inclusive), get Gaps will return zero
entry
+ // as file size is not more than 39, so there is no gap.
+ List<BlobLayout.BlobRange> gaps = cache.getGaps(eTag, 35, 45);
+ Assertions.assertThat(gaps)
+ .describedAs("List should contains 0 elements.")
+ .hasSize(0);
+ // This will return 1 entry for 35-39 -> host-d
+ List<BlobLayout.BlobRange> blobRangeList = cache.getBlobLayout(eTag, 35,
+ 45);
+ Assertions.assertThat(blobRangeList)
+ .describedAs("List should contains 1 elements.")
+ .hasSize(1);
+ Assertions.assertThat(blobRangeList.get(0).host())
+ .describedAs("Host should be host-d.")
+ .isEqualTo("host-d");
+ cache.deregisterStream(eTag);
+ }
+
+ /**
+ * Tests putting blob layouts with gaps and filling those gaps.
+ * <p>
+ * Verifies that gaps are correctly identified, filled, and the correct blob
ranges and hosts are returned.
+ */
+ @Test
+ public void testPutBlobLayoutWithGap() {
+ String eTag = new Throwable().getStackTrace()[0].getMethodName();
+ cache.registerStream(eTag, contentLength);
+ cache.putBlobLayout(eTag, layoutResponse, contentLength);
+ BlobLayoutResponse blobLayoutResponse = new BlobLayoutResponse();
+ blobLayoutResponse.setRanges(
+ List.of(new BlobLayoutResponse.Range(45, 54, 0)));
+ blobLayoutResponse.setEndpoints(
+ Set.of(new BlobLayoutResponse.Endpoint(0, "host-a")));
+ cache.putBlobLayout(eTag, blobLayoutResponse, contentLength);
+
+ // Layout present for 35-39 & 45-54.
+ // Gap: 40-44 & 55-65
+ List<BlobLayout.BlobRange> gaps = cache.getGaps(eTag, 35, 65);
+ Assertions.assertThat(gaps)
+ .describedAs("List should contains 2 elements.")
+ .hasSize(2);
+ Assertions.assertThat(gaps.get(0).start())
+ .describedAs("First gap should start from 40.")
+ .isEqualTo(40);
+ Assertions.assertThat(gaps.get(0).end())
+ .describedAs("First gap should end to 44.")
+ .isEqualTo(44);
+ Assertions.assertThat(gaps.get(1).start())
+ .describedAs("Second gap should start from 55.")
+ .isEqualTo(55);
+ Assertions.assertThat(gaps.get(1).end())
+ .describedAs("Second gap should end to 65.")
+ .isEqualTo(65);
+
+ // Gap fill
+ blobLayoutResponse = new BlobLayoutResponse();
+ blobLayoutResponse.setRanges(
+ List.of(new BlobLayoutResponse.Range(35, 44, 0)));
+ blobLayoutResponse.setEndpoints(
+ Set.of(new BlobLayoutResponse.Endpoint(0, "host-d")));
+ cache.putBlobLayout(eTag, blobLayoutResponse, contentLength);
+ gaps = cache.getGaps(eTag, 35, 65);
+ // Only one gap remaining - 55-65
+ Assertions.assertThat(gaps)
+ .describedAs("List should contains 0 elements.")
+ .hasSize(1);
+ Assertions.assertThat(gaps.get(0).start())
+ .describedAs("Gap should start from 55.")
+ .isEqualTo(55);
+ Assertions.assertThat(gaps.get(0).end())
+ .describedAs("Gap should end to 65.")
+ .isEqualTo(65);
+
+ List<BlobLayout.BlobRange> blobRanges = cache.getBlobLayout(eTag, 35, 65);
+ // 1st - 35-44 -> host-d
+ // 2nd - 45-54 -> host-a
+ Assertions.assertThat(blobRanges)
+ .describedAs("List should contains 2 elements.")
+ .hasSize(2);
+ Assertions.assertThat(blobRanges.get(0).host())
+ .describedAs("First range host should be host-d.")
+ .isEqualTo("host-d");
+ Assertions.assertThat(blobRanges.get(1).host())
+ .describedAs("Second range host should be host-a.")
+ .isEqualTo("host-a");
+ cache.deregisterStream(eTag);
+ }
+
+ /**
+ * Tests cache eviction behavior in BlobLayoutCache.
+ * <p>
+ * Verifies that after cache eviction, blob layout retrieval returns null.
+ */
+ @Test
+ public void testCacheEviction() throws InterruptedException {
Review Comment:
This sleeps 61 seconds on the shared singleton. BlobLayoutCacheEvictionTest
already has a FakeTicker, so please move this coverage there and drop the sleep
> 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]
