Skip to content

Core: HadoopTables support dynamic FileIO #4459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shidayang wants to merge 1 commit into from
Closed

Core: HadoopTables support dynamic FileIO #4459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions core/src/main/java/org/apache/iceberg/hadoop/HadoopTables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.relocated.com.google.common.annotations.VisibleForTesting;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
Expand All @@ -68,12 +69,23 @@ public class HadoopTables implements Tables, Configurable {

private Configuration conf;

private FileIO fileIO;

public HadoopTables() {
this(new Configuration());
}

public HadoopTables(FileIO fileIO) {
Copy link
Member

@openinx openinx Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain why do we need the other FileIO rather than initializing the HadoopFileIO by default ?

Copy link
Contributor Author

@shidayang shidayang Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review, My FileSystem which has some external function such as authentication is not a standard implementation. In other cases, Many Engines has owner FileSystem Manager so they don't need HadoopTables to initialize FileSystem.

Copy link
Member

@openinx openinx Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the FileSystem is not a standard HDFS implementation, then I will think the table is not a stand hadoop table. So why not just define the customized catalog & table implementation ?

Copy link
Contributor Author

@shidayang shidayang Apr 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the one hand, The only difference is the FileIO. The constructor of HadoopTableOperations is protected and constructor args has FileIO, So the design philosophy of HadoopTableOperations is support Dynamic FileIO.

In other hand, like #1618

this(new Configuration(), fileIO);
}

public HadoopTables(Configuration conf) {
this(conf, null);
}

public HadoopTables(Configuration conf, FileIO fileIO) {
this.conf = conf;
this.fileIO = fileIO;
}

/**
Expand Down Expand Up @@ -202,9 +214,9 @@ public boolean dropTable(String location, boolean purge) {
@VisibleForTesting
TableOperations newTableOps(String location) {
if (location.contains(METADATA_JSON)) {
return new StaticTableOperations(location, new HadoopFileIO(conf));
return new StaticTableOperations(location, fileIO == null? new HadoopFileIO(conf): fileIO);
} else {
return new HadoopTableOperations(new Path(location), new HadoopFileIO(conf), conf,
return new HadoopTableOperations(new Path(location), fileIO == null? new HadoopFileIO(conf): fileIO, conf,
createOrGetLockManager(this));
}
}
Expand Down
19 changes: 19 additions & 0 deletions core/src/test/java/org/apache/iceberg/hadoop/TestHadoopTables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Paths;

import org.apache.hadoop.conf.Configuration;
import org.apache.iceberg.AppendFiles;
import org.apache.iceberg.AssertHelpers;
import org.apache.iceberg.DataFile;
Expand All @@ -33,6 +35,9 @@
import org.apache.iceberg.SortOrder;
import org.apache.iceberg.Table;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.io.FileIO;
import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.io.OutputFile;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
import org.apache.iceberg.transforms.Transform;
Expand Down Expand Up @@ -164,6 +169,20 @@ public void testTableName() {
Assert.assertEquals("Name must match", location + "#snapshots", snapshotsTable.name());
}

@Test
public void testDynamicFileIO(){
FileIO testFileIO = new HadoopFileIO(new Configuration());
PartitionSpec spec = PartitionSpec.builderFor(SCHEMA)
.bucket("data", 16)
.build();
String location = tableDir.toURI().toString();
HadoopTables hadoopTables = new HadoopTables(testFileIO);
hadoopTables.create(SCHEMA, spec, location);
Table table = hadoopTables.load(location);
Assert.assertEquals("Name must match", testFileIO, table.io());
Assert.assertTrue("FileIO must match", testFileIO == table.io());
}

private static void createDummyTable(File tableDir, File dataDir) throws IOException {
Table table = TABLES.create(SCHEMA, tableDir.toURI().toString());
AppendFiles append = table.newAppend();
Expand Down