hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Charles Lamb (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-6692) Add more HDFS encryption tests
Date Wed, 30 Jul 2014 17:24:39 GMT

    [ 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:

    injector = new MyInjector(1, 1);
    EncryptionFaultInjector.instance = injector;
    future = executor.submit(new CreateFileTask(fsWrapper, file));


    assertEquals("Expected a startFile retry", 2, injector.generateCount);

So maybe the creamy filling in between those two blocks (e.g.):

    fsWrapper.delete(zone1, true);
    fsWrapper.mkdir(zone1, FsPermission.getDirDefault(), true);
    dfsAdmin.createEncryptionZone(zone1, otherKey);

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

View raw message