Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
hadoop-yetus commented on PR #6514:
URL: https://github.com/apache/hadoop/pull/6514#issuecomment-2078145607
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 01s | | No case conflicting files
found. |
| +0 :ok: | spotbugs | 0m 00s | | spotbugs executables are not
available. |
| +0 :ok: | codespell | 0m 00s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 01s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 00s | | The patch does not contain
any @author tags. |
| -1 :x: | test4tests | 0m 00s | | 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 | 107m 55s | | trunk passed |
| +1 :green_heart: | compile | 6m 03s | | trunk passed |
| +1 :green_heart: | checkstyle | 5m 16s | | trunk passed |
| +1 :green_heart: | mvnsite | 6m 32s | | trunk passed |
| +1 :green_heart: | javadoc | 5m 41s | | trunk passed |
| +1 :green_heart: | shadedclient | 172m 45s | | branch has no errors
when building and testing our client artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 44s | | the patch passed |
| +1 :green_heart: | compile | 3m 00s | | the patch passed |
| +1 :green_heart: | javac | 3m 00s | | the patch passed |
| +1 :green_heart: | blanks | 0m 00s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 2m 21s | | the patch passed |
| +1 :green_heart: | mvnsite | 3m 25s | | the patch passed |
| +1 :green_heart: | javadoc | 2m 50s | | the patch passed |
| +1 :green_heart: | shadedclient | 185m 14s | | patch has no errors
when building and testing our client artifacts. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 6m 11s | | The patch does not
generate ASF License warnings. |
| | | 494m 11s | | |
| Subsystem | Report/Notes |
|--:|:-|
| GITHUB PR | https://github.com/apache/hadoop/pull/6514 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | MINGW64_NT-10.0-17763 6320a162bf26 3.4.10-87d57229.x86_64
2024-02-14 20:17 UTC x86_64 Msys |
| Build tool | maven |
| Personality | /c/hadoop/dev-support/bin/hadoop.sh |
| git revision | trunk / 36954d34bac56942b8827587a13fed7893b51efa |
| Default Java | Azul Systems, Inc.-1.8.0_332-b09 |
| Test Results |
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6514/1/testReport/
|
| modules | C: hadoop-hdfs-project/hadoop-hdfs-client U:
hadoop-hdfs-project/hadoop-hdfs-client |
| Console output |
https://ci-hadoop.apache.org/job/hadoop-multibranch-windows-10/job/PR-6514/1/console
|
| versions | git=2.44.0.windows.1 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
Hexiaoqiao commented on PR #6514: URL: https://github.com/apache/hadoop/pull/6514#issuecomment-1984965658 > @Hexiaoqiao @zhangshuyan0 Thanks for your reviews. I realized there's also an ElasticBufferPool in DFSStripedOutputStream. I'm thinking of handling that here as well. What do you think? +1 from my side. -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
Hexiaoqiao commented on code in PR #6514:
URL: https://github.com/apache/hadoop/pull/6514#discussion_r1517110356
##
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java:
##
@@ -3159,6 +3165,21 @@ private void initThreadsNumForStripedReads(int
numThreads) {
}
}
+ private void initBufferPoolForStripedReads(boolean useWeakReference) {
+if (STRIPED_READ_BUFFER_POOL != null) {
+ return;
+}
+synchronized (DFSClient.class) {
Review Comment:
Got it.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
bbeaudreault commented on PR #6514: URL: https://github.com/apache/hadoop/pull/6514#issuecomment-1983545666 @Hexiaoqiao @zhangshuyan0 Thanks for your reviews. I realized there's also an ElasticBufferPool in DFSStripedOutputStream. I'm thinking of handling that here as well. What do you think? -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
bbeaudreault commented on code in PR #6514:
URL: https://github.com/apache/hadoop/pull/6514#discussion_r1516181907
##
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java:
##
@@ -3159,6 +3165,21 @@ private void initThreadsNumForStripedReads(int
numThreads) {
}
}
+ private void initBufferPoolForStripedReads(boolean useWeakReference) {
+if (STRIPED_READ_BUFFER_POOL != null) {
+ return;
+}
+synchronized (DFSClient.class) {
Review Comment:
@Hexiaoqiao thanks for review. For this block, it's sort of modeled after
other examples in DFSClient, such [as initializing the striped read thread
pool](https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java#L3150).
I think the idea is that DFSClient could easily be used in multiple threads,
so we want to avoid double initializing the shared resource.
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
bbeaudreault commented on code in PR #6514:
URL: https://github.com/apache/hadoop/pull/6514#discussion_r1516179674
##
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java:
##
@@ -530,6 +530,9 @@ interface StripedRead {
* span 6 DNs, so this default value accommodates 3 read streams
*/
int THREADPOOL_SIZE_DEFAULT = 18;
+
+String WEAK_REF_BUFFER_POOL_KEY = PREFIX +
"bufferpool.weak.references.enabled";
+boolean WEAK_REF_BUFFER_POOL_DEFAULT = false;
Review Comment:
Will do
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
Re: [PR] HDFS-17364. EC: Configurably use WeakReferencedElasticByteBufferPool in DFSStripedInputStream. [hadoop]
Hexiaoqiao commented on code in PR #6514:
URL: https://github.com/apache/hadoop/pull/6514#discussion_r1516099526
##
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/HdfsClientConfigKeys.java:
##
@@ -530,6 +530,9 @@ interface StripedRead {
* span 6 DNs, so this default value accommodates 3 read streams
*/
int THREADPOOL_SIZE_DEFAULT = 18;
+
+String WEAK_REF_BUFFER_POOL_KEY = PREFIX +
"bufferpool.weak.references.enabled";
+boolean WEAK_REF_BUFFER_POOL_DEFAULT = false;
Review Comment:
Please also add this default config to core-default.xml.
##
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java:
##
@@ -3159,6 +3165,21 @@ private void initThreadsNumForStripedReads(int
numThreads) {
}
}
+ private void initBufferPoolForStripedReads(boolean useWeakReference) {
+if (STRIPED_READ_BUFFER_POOL != null) {
+ return;
+}
+synchronized (DFSClient.class) {
Review Comment:
What this `synchronized` would like to protect?
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
