Skip to content
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

Core: Optimize S3 layout of Datafiles by expanding first character set of the hash #7128

Merged
merged 7 commits into from
Apr 3, 2023
Merged

Core: Optimize S3 layout of Datafiles by expanding first character set of the hash #7128

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e846bba
Core: Optimize S3 layout
Mar 15, 2023
7a04d65
address review feedback
Mar 17, 2023
6757c8a
Address review feedback
Mar 19, 2023
e70c81a
use the suggested hash function
Mar 20, 2023
42aad4e
Core: Improve bit density in object storage layout.
rdblue Apr 2, 2023
b0eb42a
fix style checks
Apr 3, 2023
024440c
remove lowe-case requriement from base64 encoder
Apr 3, 2023
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
26 changes: 18 additions & 8 deletions core/src/main/java/org/apache/iceberg/LocationProviders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,16 @@
*/
package org.apache.iceberg;

import java.nio.charset.StandardCharsets;
import java.util.Map;
import java.util.function.Function;
import org.apache.hadoop.fs.Path;
import org.apache.iceberg.common.DynConstructors;
import org.apache.iceberg.io.LocationProvider;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.transforms.Transforms;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.relocated.com.google.common.hash.HashCode;
import org.apache.iceberg.relocated.com.google.common.hash.HashFunction;
import org.apache.iceberg.relocated.com.google.common.hash.Hashing;
import org.apache.iceberg.relocated.com.google.common.io.BaseEncoding;
import org.apache.iceberg.util.LocationUtil;
import org.apache.iceberg.util.PropertyUtil;

Expand Down Expand Up @@ -104,9 +106,10 @@ public String newDataLocation(String filename) {
}

static class ObjectStoreLocationProvider implements LocationProvider {
private static final Function<Object, Integer> HASH_FUNC =
Transforms.bucket(Integer.MAX_VALUE).bind(Types.StringType.get());

private static final HashFunction HASH_FUNC = Hashing.murmur3_32_fixed();
private static final BaseEncoding BASE64_ENCODER = BaseEncoding.base64Url().omitPadding();
private final ThreadLocal<byte[]> temp = ThreadLocal.withInitial(() -> new byte[4]);
private final String storageLocation;
private final String context;

Expand Down Expand Up @@ -143,11 +146,11 @@ public String newDataLocation(PartitionSpec spec, StructLike partitionData, Stri

@Override
public String newDataLocation(String filename) {
int hash = HASH_FUNC.apply(filename);
String hash = computeHash(filename);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to support hash type as a configuration ? This change might break if anyone having custom list operation based from object store on hex prefix (%08x).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that's a good idea. If someone has a custom list operation, then they're relying on behaviors of the implementation and take the risk that something like this will break their implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you mean by custom list operation? Listing partitions based on the path after the random prefix?

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @jackye1995 if a object storage path has millions of files, S3 listing usually takes very long time since it has to go thru all the keys recursively. This behavior can be improved by listing files with hash prefix in parallel like /data/00, /data/01, /data/0a etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for what Daniel says.

Copy link
Contributor

Choose a reason for hiding this comment

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

if a object storage path has millions of files, S3 listing usually takes very long time since it has to go thru all the keys recursively.

do you mean during orphan file removal? There could be better ways in making that work without listing if it's considered slow, like running a join against the inventory list.

if (context != null) {
return String.format("%s/%08x/%s/%s", storageLocation, hash, context, filename);
return String.format("%s/%s/%s/%s", storageLocation, hash, context, filename);
} else {
return String.format("%s/%08x/%s", storageLocation, hash, filename);
return String.format("%s/%s/%s", storageLocation, hash, filename);
}
}

Expand All @@ -167,5 +170,12 @@ private static String pathContext(String tableLocation) {

return resolvedContext;
}

private String computeHash(String fileName) {
byte[] bytes = temp.get();
HashCode hash = HASH_FUNC.hashString(fileName, StandardCharsets.UTF_8);
hash.writeBytesTo(bytes, 0, 4);
return BASE64_ENCODER.encode(bytes);
}
}
}