[
https://issues.apache.org/jira/browse/HADOOP-18399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17638641#comment-17638641
]
ASF GitHub Bot commented on HADOOP-18399:
-----------------------------------------
steveloughran commented on code in PR #5054:
URL: https://github.com/apache/hadoop/pull/5054#discussion_r1032391988
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/prefetch/S3APrefetchFakes.java:
##########
@@ -335,7 +341,8 @@ protected void writeFile(Path path, ByteBuffer buffer)
throws IOException {
private long fileCount = 0;
@Override
- protected Path getCacheFilePath() throws IOException {
+ protected Path getCacheFilePath(Configuration conf, LocalDirAllocator
localDirAllocator)
Review Comment:
think this should be indented more
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -1561,7 +1561,7 @@ private FSDataInputStream executeOpen(
readContext.build(),
createObjectAttributes(path, fileStatus),
createInputStreamCallbacks(auditSpan),
- inputStreamStats));
+ inputStreamStats, getConf()));
Review Comment:
put on a new line for consistency
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3APrefetchingInputStream.java:
##########
@@ -60,8 +61,7 @@ public ITestS3APrefetchingInputStream() {
super(true);
}
- private static final Logger LOG =
- LoggerFactory.getLogger(ITestS3APrefetchingInputStream.class);
+ private static final Logger LOG =
LoggerFactory.getLogger(ITestS3APrefetchingInputStream.class);
Review Comment:
not needed; just revert to make patch smaller. unless you really, really
want this
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/impl/prefetch/CachingBlockManager.java:
##########
@@ -129,6 +138,8 @@ public CachingBlockManager(
this.ops = new BlockOperations();
this.ops.setDebug(false);
+ this.conf = conf;
Review Comment:
add requireNonNull() checks here
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3APrefetchingInputStream.java:
##########
@@ -294,4 +294,34 @@ public void testStatusProbesAfterClosingStream() throws
Throwable {
}
+ @Test
+ public void testCacheFileExistence() throws Throwable {
Review Comment:
is there any cleanup of these files after? i know a maven clean will do it,
but these may be be big files and create problems on docker container runs.
best to do an rm of the dir, or at least the files, afterwards
> SingleFilePerBlockCache to use LocalDirAllocator for file allocatoin
> --------------------------------------------------------------------
>
> Key: HADOOP-18399
> URL: https://issues.apache.org/jira/browse/HADOOP-18399
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Steve Loughran
> Assignee: Viraj Jasani
> Priority: Major
> Labels: pull-request-available
>
> prefetching stream's SingleFilePerBlockCache uses Files.tempFile() to
> allocate a temp file.
> it should be using LocalDirAllocator to allocate space from a list of dirs,
> taking a config key to use. for s3a we will use the Constants.BUFFER_DIR
> option, which on yarn deployments is fixed under the env.LOCAL_DIR path, so
> automatically cleaned up on container exit
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
