hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Colin Patrick McCabe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-9188) Make block corruption related tests FsDataset-agnostic.
Date Thu, 08 Oct 2015 20:05:27 GMT

    [ https://issues.apache.org/jira/browse/HDFS-9188?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14949284#comment-14949284
] 

Colin Patrick McCabe commented on HDFS-9188:
--------------------------------------------

Thanks, [~eddyxu].

{code}
38	  abstract class Factory<D extends FsDatasetTestUtils> {
39	    /**
40	     * By default returns the Flood version of the factory.
41	     *
42	     * @return The configured Factory.
43	     */
{code}
This comment seems out of place.

{code}
26	public class FsDatasetTestUtilsFactory
27	    extends FsDatasetTestUtils.Factory<FsDatasetTestUtils> {
28	  @Override
29	  public FsDatasetTestUtils newInstance(DataNode datanode) {
30	    return new FsDatasetImplTestUtils(datanode);
31	  }
{code}
The factory name should be {{FsDatasetImplTestUtilsFactory}}, since it's returning an instance
of {{FsDatasetImplTestUtils}}.  Also, this should be {{final}}.

{code}
60    private static boolean corrupt(File file) throws IOException {
61	      if (file == null || !file.exists()) {
62	        return false;
63	      }
64	      LOG.debug("Corrupting file: " + file);
65	      StringBuilder content = new StringBuilder(DFSTestUtil.readFile(file));
66	      char c = content.charAt(0);
67	      content.setCharAt(0, ++c);
68	      try (PrintWriter out = new PrintWriter(file)) {
69	        out.print(content);
70	        out.flush();
71	      }
72	      return true;
{code}
This is assuming that the content is a string.  It would be better to just manipulate the
file as a binary array.  This also doesn't specify the string encoding (unfortunately, we
can't take UTF-8 for granted here).

{code}
88	    /** Corrupt a block / crc file by deleting it. */
89	    private static boolean delete(File file) {
90	      if (file == null || !file.exists()) {
91	        return false;
92	      }
93	      return file.delete();
94	    }
{code}
Please use {{Files#delete}}, which throws an exception when it fails.  And don't return a
boolean.  See http://docs.oracle.com/javase/7/docs/api/java/nio/file/Files.html#delete(java.nio.file.Path)

{code}
101	    @Override
102	    public boolean corruptData() throws IOException {
103	      LOG.info("Corrupting block file: " + blockFile);
104	      return corrupt(blockFile);
105	    }
{code}
Why does this return a boolean?  If it fails it should throw an exception with the reason
why it failed.  Same question with truncate.

If we need a {{MaterializedReplica#exists}} method then let's add one rather than making everything
return false if the replica doesn't exist.

> Make block corruption related tests FsDataset-agnostic. 
> --------------------------------------------------------
>
>                 Key: HDFS-9188
>                 URL: https://issues.apache.org/jira/browse/HDFS-9188
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: HDFS, test
>    Affects Versions: 2.7.1
>            Reporter: Lei (Eddy) Xu
>            Assignee: Lei (Eddy) Xu
>         Attachments: HDFS-9188.000.patch, HDFS-9188.001.patch, HDFS-9188.002.patch, HDFS-9188.003.patch
>
>
> Currently, HDFS does block corruption tests by directly accessing the files stored on
the storage directories, which assumes {{FsDatasetImpl}} is the dataset implementation. However,
with works like OZone (HDFS-7240) and HDFS-8679, there will be different FsDataset implementations.

> So we need a general way to run whitebox tests like corrupting blocks and crc files.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message