[
https://issues.apache.org/jira/browse/FLUME-2653?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15386441#comment-15386441
]
Mike Percy commented on FLUME-2653:
-----------------------------------
This looks like a useful change.
Thanks for the initial review pass, [~bessbd]. I have some additional comments
about the patch below.
[~bimaltandel], thank you for the patch. Sorry it took so long for it to be
reviewed.
Here is some review feedback on your patch:
{code}
+ private boolean UseSuffix = true;
{code}
Please use Java style for variables, which must be named with camelCaps style.
Flume uses the Google Java code style. See
https://google.github.io/styleguide/javaguide.html#s5.2.5-non-constant-field-names
In addition, please change the name of this variable to {{inUseSuffixEnabled}},
since I think that would be more consistent and clear.
For your "if-else" statement, you need to add brackets around the code blocks.
Please see
https://google.github.io/styleguide/javaguide.html#s4.1.1-braces-always-used
Also, I would like to see docs and a test for this new feature.
Finally, I have not looked closely, but the fact that existing tests pass
scares me. We rename temp files to the new filename when the file is rolled.
However, trying to "rename" a file from a.txt to a.txt (i.e. rename to the same
name) should fail... right? So I am skeptical of the correctness of this patch.
> Allow inUseSuffix to be null/empty
> ----------------------------------
>
> Key: FLUME-2653
> URL: https://issues.apache.org/jira/browse/FLUME-2653
> Project: Flume
> Issue Type: Improvement
> Components: Sinks+Sources
> Affects Versions: v1.5.1
> Reporter: Andrew Jones
> Assignee: bimal tandel
> Labels: docs-missing, hdfssink
> Fix For: v1.7.0
>
> Attachments: FLUME-2653.patch
>
>
> At the moment, it doesn't seem possible to set the null/empty. We've tried
> {{''}} which just adds the quotes to the end, and setting to nothing, which
> just uses the default {{.tmp}}.
> We want the _in use_ file to have the same name as the _closed_ file, so we
> can read from files that are in use without the file moving from underneath
> us. In our use case, we know that an in use file is still readable and
> parseable, because it is just text with a JSON document per line.
> It looks like [the HDFS sink
> code|https://github.com/apache/flume/blob/542b1695033d330eb00ae81713fdc838b88332b6/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L618]
> can handle this change already, but at the moment there is no way to set the
> {{bucketPath}} and {{targetPath}} to be the same.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
