[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2024-01-20 Thread Shilun Fan (Jira)


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

Shilun Fan commented on HADOOP-18740:
-

3.3.6 release has been fixed, fix version removed 3.4.0

> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Affects Versions: 3.3.6
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
> Fix For: 3.3.6
>
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-06-07 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

steveloughran merged PR #5675:
URL: https://github.com/apache/hadoop/pull/5675




> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-06-01 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

hadoop-yetus commented on PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#issuecomment-1572596503

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 49s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | -1 :x: |  test4tests  |   0m  0s |  |  The patch doesn't appear to include 
any new or modified tests. Please justify why no new tests are needed for this 
patch. Also please list what manual steps were performed to verify this patch.  
|
    _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  37m 27s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  17m 27s |  |  trunk passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  compile  |  15m 47s |  |  trunk passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 30s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m  9s |  |  trunk passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  |  trunk passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  spotbugs  |   2m 42s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  25m 22s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 51s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  16m 42s |  |  the patch passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  javac  |  16m 42s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  15m 45s |  |  the patch passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  javac  |  15m 45s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  0s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 31s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  0s |  |  the patch passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  |  the patch passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  spotbugs  |   2m 33s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  24m 50s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 22s |  |  hadoop-common in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   0m 53s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 189m 16s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5675/3/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/5675 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux ce03b807e480 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 
19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / d2b9210104b1ec0e4a09f22fcfc6df7b82557017 |
   | Default Java | Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5675/3/testReport/ |
   | Max. process+thread count | 1244 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: 
hadoop-common-project/hadoop-common |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5675/3/console |
   | versions | git=2.25.1 maven=3.6.3 spotbu

[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-06-01 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

virajjasani commented on code in PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#discussion_r1213340095


##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -127,11 +128,33 @@ void takeLock(LockType lockType) {
  */
 void releaseLock(LockType lockType) {
   if (LockType.READ == lockType) {
-this.lock.readLock().unlock();
+lock.readLock().unlock();
   } else if (LockType.WRITE == lockType) {
-this.lock.writeLock().unlock();
+lock.writeLock().unlock();
   }
 }
+
+/**
+ * Try to take the read or write lock within the given timeout.
+ *
+ * @param lockType type of the lock.
+ * @param timeout the time to wait for the given lock.
+ * @param unit the time unit of the timeout argument.
+ * @return true if the lock of the given lock type was acquired.
+ */
+boolean takeLock(LockType lockType, long timeout, TimeUnit unit) {

Review Comment:
   sounds good, done





> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-06-01 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

virajjasani commented on code in PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#discussion_r1213323650


##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -310,7 +333,12 @@ public void close() throws IOException {
 int numFilesDeleted = 0;
 
 for (Entry entry : blocks.values()) {
-  entry.takeLock(Entry.LockType.WRITE);
+  boolean lockAcquired = entry.takeLock(Entry.LockType.WRITE, 5, 
TimeUnit.SECONDS);

Review Comment:
   my bad, let me fix this real quick





> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-06-01 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

steveloughran commented on code in PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#discussion_r1213038983


##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -310,7 +333,12 @@ public void close() throws IOException {
 int numFilesDeleted = 0;
 
 for (Entry entry : blocks.values()) {
-  entry.takeLock(Entry.LockType.WRITE);
+  boolean lockAcquired = entry.takeLock(Entry.LockType.WRITE, 5, 
TimeUnit.SECONDS);

Review Comment:
   pull the number into a constant. Know that I automatically -1 all inline 
constants in production code and save time all round.



##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -127,11 +128,33 @@ void takeLock(LockType lockType) {
  */
 void releaseLock(LockType lockType) {
   if (LockType.READ == lockType) {
-this.lock.readLock().unlock();
+lock.readLock().unlock();
   } else if (LockType.WRITE == lockType) {
-this.lock.writeLock().unlock();
+lock.writeLock().unlock();
   }
 }
+
+/**
+ * Try to take the read or write lock within the given timeout.
+ *
+ * @param lockType type of the lock.
+ * @param timeout the time to wait for the given lock.
+ * @param unit the time unit of the timeout argument.
+ * @return true if the lock of the given lock type was acquired.
+ */
+boolean takeLock(LockType lockType, long timeout, TimeUnit unit) {

Review Comment:
   should this and the others be private? you don't want other classes playing 
with your lock code...





> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-05-31 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

hadoop-yetus commented on PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#issuecomment-1571346917

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 33s |  |  Docker mode activated.  |
    _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files 
found.  |
   | +0 :ok: |  codespell  |   0m  1s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  1s |  |  detect-secrets was not available.  
|
   | +1 :green_heart: |  @author  |   0m  0s |  |  The patch does not contain 
any @author tags.  |
   | -1 :x: |  test4tests  |   0m  0s |  |  The patch doesn't appear to include 
any new or modified tests. Please justify why no new tests are needed for this 
patch. Also please list what manual steps were performed to verify this patch.  
|
    _ trunk Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  34m  1s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |  15m 37s |  |  trunk passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  compile  |  14m 21s |  |  trunk passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   1m 35s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 17s |  |  trunk passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 52s |  |  trunk passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  spotbugs  |   2m 41s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  22m  8s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 49s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  14m 52s |  |  the patch passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  javac  |  14m 52s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |  14m 19s |  |  the patch passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  javac  |  14m 19s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   1m 33s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   1m  5s |  |  the patch passed with JDK 
Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  |  the patch passed with JDK 
Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09  |
   | +1 :green_heart: |  spotbugs  |   2m 34s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  22m 23s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  |  18m 32s |  |  hadoop-common in the patch 
passed.  |
   | +1 :green_heart: |  asflicense  |   1m  2s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 174m 48s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.43 ServerAPI=1.43 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5675/2/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/5675 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
   | uname | Linux 9432f0056538 4.15.0-206-generic #217-Ubuntu SMP Fri Feb 3 
19:10:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 710441f19ca08701a4e0f5c32fa77f3ff7ce9736 |
   | Default Java | Private Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.19+7-post-Ubuntu-0ubuntu120.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_362-8u372-ga~us1-0ubuntu1~20.04-b09 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5675/2/testReport/ |
   | Max. process+thread count | 1783 (vs. ulimit of 5500) |
   | modules | C: hadoop-common-project/hadoop-common U: 
hadoop-common-project/hadoop-common |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-5675/2/console |
   | versions | git=2.25.1 maven=3.6.3 spotbu

[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-05-31 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

virajjasani commented on PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#issuecomment-1571320810

   `us-west-2`:
   
   ```
   mvn clean verify -Dparallel-tests -DtestsThreadCount=8 -Dscale
   
   mvn clean verify -Dparallel-tests -DtestsThreadCount=8 -Dscale -Dprefetch
   ```
   
   Test results are clean




> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-05-31 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

virajjasani commented on code in PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#discussion_r1212472590


##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -268,12 +310,15 @@ public void close() throws IOException {
 int numFilesDeleted = 0;
 
 for (Entry entry : blocks.values()) {
+  entry.takeLock(Entry.LockType.WRITE);

Review Comment:
   > also, L303: should closed be atomic?
   
   +1 to this suggestion, let me create a separate patch with HADOOP-18756 to 
better track it.
   
   > good: no race condition in close
   > bad) the usual
   
   sounds reasonable, let me try setting timeout





> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-05-31 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

steveloughran commented on code in PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#discussion_r1211811893


##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -99,6 +106,32 @@ public String toString() {
   "([%03d] %s: size = %d, checksum = %d)",
   blockNumber, path, size, checksum);
 }
+
+/**
+ * Take the read or write lock.
+ *
+ * @param lockType type of the lock.
+ */
+void takeLock(LockType lockType) {
+  if (LockType.READ == lockType) {
+this.lock.readLock().lock();

Review Comment:
   remove the `this.`



##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -99,6 +106,32 @@ public String toString() {
   "([%03d] %s: size = %d, checksum = %d)",
   blockNumber, path, size, checksum);
 }
+
+/**
+ * Take the read or write lock.
+ *
+ * @param lockType type of the lock.
+ */
+void takeLock(LockType lockType) {
+  if (LockType.READ == lockType) {
+this.lock.readLock().lock();
+  } else if (LockType.WRITE == lockType) {
+this.lock.writeLock().lock();
+  }
+}
+
+/**
+ * Release the read or write lock.
+ *
+ * @param lockType type of the lock.
+ */
+void releaseLock(LockType lockType) {
+  if (LockType.READ == lockType) {
+this.lock.readLock().unlock();

Review Comment:
   remove the `this.`



##
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/SingleFilePerBlockCache.java:
##
@@ -268,12 +310,15 @@ public void close() throws IOException {
 int numFilesDeleted = 0;
 
 for (Entry entry : blocks.values()) {
+  entry.takeLock(Entry.LockType.WRITE);

Review Comment:
   should we be acquiring locks in close()?
   good: no race condition in close
   bad) the usual
   
   also, L303: should closed be atomic?





> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-05-31 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

steveloughran commented on PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#issuecomment-1570335711

   @MayankSinghParmar get on the hadoop hdfs mailing list and discuss there.
   
   @virajjasani can you rebase and retest?




> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>  Labels: pull-request-available
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



[jira] [Commented] (HADOOP-18740) s3a prefetch cache blocks should be accessed by RW locks

2023-05-30 Thread ASF GitHub Bot (Jira)


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

ASF GitHub Bot commented on HADOOP-18740:
-

MayankSinghParmar commented on PR #5675:
URL: https://github.com/apache/hadoop/pull/5675#issuecomment-1568970535

   I want to implement a new cache replacement algorithm in hdfs but I don't 
know which part of module to use and modify. Can any one please help me find 
the modules and functions which need to override to add my own caching 
replacement algorithm in hdfs




> s3a prefetch cache blocks should be accessed by RW locks
> 
>
> Key: HADOOP-18740
> URL: https://issues.apache.org/jira/browse/HADOOP-18740
> Project: Hadoop Common
>  Issue Type: Sub-task
>Reporter: Viraj Jasani
>Assignee: Viraj Jasani
>Priority: Major
>
> In order to implement LRU or LFU based cache removal policies for s3a 
> prefetched cache blocks, it is important for all cache reader threads to 
> acquire read lock and similarly cache file removal mechanism (fs close or 
> cache eviction) to acquire write lock before accessing the files.
> As we maintain the block entries in an in-memory map, we should be able to 
> introduce read-write lock per cache file entry, we don't need coarse-grained 
> lock shared by all entries.
>  
> This is a prerequisite to HADOOP-18291.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org