[
https://issues.apache.org/jira/browse/HDFS-6692?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14079577#comment-14079577
]
Charles Lamb commented on HDFS-6692:
------------------------------------
This is a nice piece of work. My comments are stylistic.
* Extra newline at the end of MyInjector, probably IDE induced.
* Why do you access EFI.instance directly? Doesn't FB whine at you about having
a public instance variable that you're accessing directly, or do we disable
that check?
* MyInjector is always init'd with 1,1. Perhaps just make a no-args ctor
instead?
* Naming. EncryptionFaultInjector is a relatively general name, and
startFileAfterGenerateKey is more specific. But I wonder if this should be
turned into a more general facility, including naming. I'm thinking something
like:
TestFaultInjector or DebugFaultInjector instead of EncryptionFaultInjector. And
then perhaps startFileAfterGenerateKey could be renamed to injectFault or just
inject. generateCount is more like an injectCount or injectionCount or
callCount or just count.
* If you drink that glass of Kool-Aid and agree that things could be more
generally named, then you'll perhaps buy into making this a slightly more
general, reusable facility (DFSTestUtil). There's repeated code:
{code}
injector = new MyInjector(1, 1);
EncryptionFaultInjector.instance = injector;
future = executor.submit(new CreateFileTask(fsWrapper, file));
injector.ready.await();
{code}
and
{code}
injector.wait.countDown();
future.get();
assertEquals("Expected a startFile retry", 2, injector.generateCount);
{code}
So maybe the creamy filling in between those two blocks (e.g.):
{code}
fsWrapper.delete(zone1, true);
fsWrapper.mkdir(zone1, FsPermission.getDirDefault(), true);
dfsAdmin.createEncryptionZone(zone1, otherKey);
{code}
could be turned into a Callable?
Of course, the fly in the ointment is the last test ("Flip-flop between two EZs
to repeatedly fail") which doesn't exactly follow this pattern, so maybe this
is unattainable. I'm happy with this being a follow-on Jira if you think it's a
worthwhile endeavour.
> Add more HDFS encryption tests
> ------------------------------
>
> Key: HDFS-6692
> URL: https://issues.apache.org/jira/browse/HDFS-6692
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: security
> Affects Versions: fs-encryption (HADOOP-10150 and HDFS-6134)
> Reporter: Andrew Wang
> Assignee: Andrew Wang
> Attachments: hdfs-6692.001.patch, hdfs-6692.002.patch
>
>
> Now that we have the basic pieces in place for encryption, it's a good time
> to look at our test coverage and add new tests.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
