yihua commented on code in PR #12866: URL: https://github.com/apache/hudi/pull/12866#discussion_r1978102602
########## hudi-io/src/main/java/org/apache/hudi/io/hfile/writer/ChecksumType.java: ########## @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hudi.io.hfile.writer; + +import org.apache.yetus.audience.InterfaceAudience; + [email protected] Review Comment: Remove this annotation? ########## hudi-io/src/main/java/org/apache/hudi/io/hfile/writer/Block.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hudi.io.hfile.writer; + +import org.apache.hudi.io.hfile.HFileBlock; + +import com.google.protobuf.CodedOutputStream; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; + +public abstract class Block { Review Comment: Could this class be merged with `HFileBlock`? ########## hudi-io/src/main/java/org/apache/hudi/io/compress/CompressionCodec.java: ########## @@ -41,4 +41,14 @@ public enum CompressionCodec { public String getName() { return name; } + + public static CompressionCodec findCodecByName(String name) { + for (CompressionCodec v : CompressionCodec.values()) { + if (v.getName().equals(name.toLowerCase())) { + return v; + } + } + throw new IllegalArgumentException( + "Cannot find compression codec: " + name); + } Review Comment: Store a map and look it up like `decodeCompressionCodec`? ########## hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileContext.java: ########## @@ -29,37 +29,50 @@ public class HFileContext { private final CompressionCodec compressionCodec; private final HoodieDecompressor decompressor; + private final int blockSize; - private HFileContext(CompressionCodec compressionCodec) { + private HFileContext(CompressionCodec compressionCodec, int blockSize) { this.compressionCodec = compressionCodec; this.decompressor = HoodieDecompressorFactory.getDecompressor(compressionCodec); + this.blockSize = blockSize; } - CompressionCodec getCompressionCodec() { + public CompressionCodec getCompressionCodec() { return compressionCodec; } - HoodieDecompressor getDecompressor() { + public HoodieDecompressor getDecompressor() { return decompressor; } + public int getBlockSize() { + return blockSize; + } + public static Builder builder() { return new Builder(); } + // TODO: Add all necessary fileds as HBASE HFileContext. Review Comment: Is this fixed? ########## hudi-io/src/main/java/org/apache/hudi/io/hfile/writer/Block.java: ########## @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hudi.io.hfile.writer; + +import org.apache.hudi.io.hfile.HFileBlock; + +import com.google.protobuf.CodedOutputStream; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.ByteBuffer; + +public abstract class Block { + public static final int DEFAULT_BYTES_PER_CHECKSUM = 16 * 1024; + public static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; + public static final ChecksumType CHECKSUM_TYPE = ChecksumType.NULL; + protected final byte[] blockMagic; + protected long blockOffset = -1; + protected int blockSize; + + protected Block(byte[] magic, int blockSize) { Review Comment: Use `HFileBlockType` instead of `magic`? ########## hudi-io/src/main/java/org/apache/hudi/io/hfile/HFileContext.java: ########## @@ -29,37 +29,50 @@ public class HFileContext { private final CompressionCodec compressionCodec; private final HoodieDecompressor decompressor; + private final int blockSize; - private HFileContext(CompressionCodec compressionCodec) { + private HFileContext(CompressionCodec compressionCodec, int blockSize) { this.compressionCodec = compressionCodec; this.decompressor = HoodieDecompressorFactory.getDecompressor(compressionCodec); + this.blockSize = blockSize; } - CompressionCodec getCompressionCodec() { + public CompressionCodec getCompressionCodec() { return compressionCodec; } - HoodieDecompressor getDecompressor() { + public HoodieDecompressor getDecompressor() { return decompressor; } + public int getBlockSize() { + return blockSize; + } + public static Builder builder() { return new Builder(); } + // TODO: Add all necessary fileds as HBASE HFileContext. public static class Builder { private CompressionCodec compressionCodec = CompressionCodec.NONE; + private int blockSize = 1024 * 1024; public Builder() { } - public Builder compressionCodec(CompressionCodec compressionCodec) { + public Builder withBlockSize(int blockSize) { + this.blockSize = blockSize; + return this; + } + + public Builder withCompressionCodec(CompressionCodec compressionCodec) { Review Comment: ```suggestion public Builder blockSize(int blockSize) { this.blockSize = blockSize; return this; } public Builder compressionCodec(CompressionCodec compressionCodec) { ``` ########## hudi-io/src/main/java/org/apache/hudi/io/hfile/writer/DataBlock.java: ########## @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hudi.io.hfile.writer; + +import org.apache.hudi.io.hfile.HFileBlockType; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; + +import static org.apache.hudi.io.hfile.DataSize.SIZEOF_INT16; + +public class DataBlock extends Block { Review Comment: Similar for this and other Block classes to be merged with existing once for easier code maintenance, i.e., merged with `HFileDataBlock` for `DataBlock`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
