Hadoop Common
  1. Hadoop Common
  2. HADOOP-7870

fix SequenceFile#createWriter with boolean createParent arg to respect createParent.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0.0
    • Fix Version/s: 1.1.0, 0.23.1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      After HBASE-6840, one set of calls to createNonRecursive(...) seems fishy - the new boolean createParent variable from the signature isn't used at all.

      +  public static Writer
      +    createWriter(FileSystem fs, Configuration conf, Path name,
      +                 Class keyClass, Class valClass, int bufferSize,
      +                 short replication, long blockSize, boolean createParent,
      +                 CompressionType compressionType, CompressionCodec codec,
      +                 Metadata metadata) throws IOException {
      +    if ((codec instanceof GzipCodec) &&
      +        !NativeCodeLoader.isNativeCodeLoaded() &&
      +        !ZlibFactory.isNativeZlibLoaded(conf)) {
      +      throw new IllegalArgumentException("SequenceFile doesn't work with " +
      +                                         "GzipCodec without native-hadoop code!");
      +    }
      +
      +    switch (compressionType) {
      +    case NONE:
      +      return new Writer(conf, 
      +          fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
      +          keyClass, valClass, metadata).ownStream();
      +    case RECORD:
      +      return new RecordCompressWriter(conf, 
      +          fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
      +          keyClass, valClass, codec, metadata).ownStream();
      +    case BLOCK:
      +      return new BlockCompressWriter(conf,
      +          fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
      +          keyClass, valClass, codec, metadata).ownStream();
      +    default:
      +      return null;
      +    }
      +  } 
      +
      

      Nicolas Spiegelberg suggests changing it to

       
      if (createParent) { use fs.create() } 
      else { use fs.createNonRecursive(); }
      
      1. hadoop-7870.patch
        3 kB
        Jonathan Hsieh
      2. hadoop-7870.patch
        3 kB
        Eli Collins
      3. hadoop-7870-1.patch
        2 kB
        Eli Collins

        Issue Links

          Activity

          Jonathan Hsieh created issue -
          Jonathan Hsieh made changes -
          Field Original Value New Value
          Description After HBASE-6840, one set of calls to createNonRecursive(...) seems fishy - the new boolean createParent variable from the signature isn't used at all.

          {code}
          + public static Writer
          + createWriter(FileSystem fs, Configuration conf, Path name,
          + Class keyClass, Class valClass, int bufferSize,
          + short replication, long blockSize, boolean createParent,
          + CompressionType compressionType, CompressionCodec codec,
          + Metadata metadata) throws IOException {
          + if ((codec instanceof GzipCodec) &&
          + !NativeCodeLoader.isNativeCodeLoaded() &&
          + !ZlibFactory.isNativeZlibLoaded(conf)) {
          + throw new IllegalArgumentException("SequenceFile doesn't work with " +
          + "GzipCodec without native-hadoop code!");
          + }
          +
          + switch (compressionType) {
          + case NONE:
          + return new Writer(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, metadata).ownStream();
          + case RECORD:
          + return new RecordCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + case BLOCK:
          + return new BlockCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + default:
          + return null;
          + }
          + }
          +
          {code}

          Nicolas Spiegelberg suggest changing it to
          {code}
          if (createParent) { use fs.createNonRecursive(); }
          else { use fs.create() }
          {code}
          After HBASE-6840, one set of calls to createNonRecursive(...) seems fishy - the new boolean createParent variable from the signature isn't used at all.

          {code}
          + public static Writer
          + createWriter(FileSystem fs, Configuration conf, Path name,
          + Class keyClass, Class valClass, int bufferSize,
          + short replication, long blockSize, boolean createParent,
          + CompressionType compressionType, CompressionCodec codec,
          + Metadata metadata) throws IOException {
          + if ((codec instanceof GzipCodec) &&
          + !NativeCodeLoader.isNativeCodeLoaded() &&
          + !ZlibFactory.isNativeZlibLoaded(conf)) {
          + throw new IllegalArgumentException("SequenceFile doesn't work with " +
          + "GzipCodec without native-hadoop code!");
          + }
          +
          + switch (compressionType) {
          + case NONE:
          + return new Writer(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, metadata).ownStream();
          + case RECORD:
          + return new RecordCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + case BLOCK:
          + return new BlockCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + default:
          + return null;
          + }
          + }
          +
          {code}

          Nicolas Spiegelberg suggests changing it to
          {code}
          if (createParent) { use fs.createNonRecursive(); }
          else { use fs.create() }
          {code}
          Jonathan Hsieh made changes -
          Link This issue depends on HADOOP-6840 [ HADOOP-6840 ]
          Jonathan Hsieh made changes -
          Description After HBASE-6840, one set of calls to createNonRecursive(...) seems fishy - the new boolean createParent variable from the signature isn't used at all.

          {code}
          + public static Writer
          + createWriter(FileSystem fs, Configuration conf, Path name,
          + Class keyClass, Class valClass, int bufferSize,
          + short replication, long blockSize, boolean createParent,
          + CompressionType compressionType, CompressionCodec codec,
          + Metadata metadata) throws IOException {
          + if ((codec instanceof GzipCodec) &&
          + !NativeCodeLoader.isNativeCodeLoaded() &&
          + !ZlibFactory.isNativeZlibLoaded(conf)) {
          + throw new IllegalArgumentException("SequenceFile doesn't work with " +
          + "GzipCodec without native-hadoop code!");
          + }
          +
          + switch (compressionType) {
          + case NONE:
          + return new Writer(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, metadata).ownStream();
          + case RECORD:
          + return new RecordCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + case BLOCK:
          + return new BlockCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + default:
          + return null;
          + }
          + }
          +
          {code}

          Nicolas Spiegelberg suggests changing it to
          {code}
          if (createParent) { use fs.createNonRecursive(); }
          else { use fs.create() }
          {code}
          After HBASE-6840, one set of calls to createNonRecursive(...) seems fishy - the new boolean createParent variable from the signature isn't used at all.

          {code}
          + public static Writer
          + createWriter(FileSystem fs, Configuration conf, Path name,
          + Class keyClass, Class valClass, int bufferSize,
          + short replication, long blockSize, boolean createParent,
          + CompressionType compressionType, CompressionCodec codec,
          + Metadata metadata) throws IOException {
          + if ((codec instanceof GzipCodec) &&
          + !NativeCodeLoader.isNativeCodeLoaded() &&
          + !ZlibFactory.isNativeZlibLoaded(conf)) {
          + throw new IllegalArgumentException("SequenceFile doesn't work with " +
          + "GzipCodec without native-hadoop code!");
          + }
          +
          + switch (compressionType) {
          + case NONE:
          + return new Writer(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, metadata).ownStream();
          + case RECORD:
          + return new RecordCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + case BLOCK:
          + return new BlockCompressWriter(conf,
          + fs.createNonRecursive(name, true, bufferSize, replication, blockSize, null),
          + keyClass, valClass, codec, metadata).ownStream();
          + default:
          + return null;
          + }
          + }
          +
          {code}

          Nicolas Spiegelberg suggests changing it to
          {code}
          if (createParent) { use fs.create() }
          else { use fs.createNonRecursive(); }
          {code}
          Jonathan Hsieh made changes -
          Assignee Jonathan Hsieh [ jmhsieh ]
          Jonathan Hsieh made changes -
          Affects Version/s 1.0.0 [ 12318244 ]
          Jonathan Hsieh made changes -
          Attachment hadoop-7870.patch [ 12505639 ]
          Jonathan Hsieh made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Eli Collins made changes -
          Attachment hadoop-7870.patch [ 12505691 ]
          Eli Collins made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Fix Version/s 1.1.0 [ 12316501 ]
          Resolution Fixed [ 1 ]
          Eli Collins made changes -
          Attachment hadoop-7870-1.patch [ 12506507 ]
          Eli Collins made changes -
          Target Version/s 0.23.1 [ 12318884 ]
          Eli Collins made changes -
          Fix Version/s 0.23.1 [ 12318884 ]
          Target Version/s 0.23.1 [ 12318884 ]
          Arun C Murthy made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Gavin made changes -
          Link This issue depends on HADOOP-6840 [ HADOOP-6840 ]
          Gavin made changes -
          Link This issue depends upon HADOOP-6840 [ HADOOP-6840 ]

            People

            • Assignee:
              Jonathan Hsieh
              Reporter:
              Jonathan Hsieh
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development