nastra commented on code in PR #12984:
URL: https://github.com/apache/iceberg/pull/12984#discussion_r2076845190
##########
aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java:
##########
@@ -98,7 +99,7 @@ public AwsClientProperties() {
}
public AwsClientProperties(Map<String, String> properties) {
- this.allProperties = SerializableMap.copyOf(properties);
+ this.allProperties = new HashMap<>(properties);
Review Comment:
in fact we're using `SerializableMap` mainly due to Kryo ser/de so I'm
surprised that this ended up being an issue.
I've also added
```
@Test
public void testKryoSerialization() throws IOException {
AwsClientProperties props =
new AwsClientProperties(
ImmutableMap.of(AwsClientProperties.CLIENT_REGION, "us-east-1",
"k1", "v1"));
AwsClientProperties deserialized =
TestHelpers.KryoHelpers.roundTripSerialize(props);
assertThat(deserialized.clientRegion()).isEqualTo(props.clientRegion());
}
```
to `AwsClientPropertiesTest` but that passes with Kryo ser/de.
If you e.g. run this test but change `allProperties` to be an
`ImmutableMap`, then you would see that `allProperties` is actually the
underlying issue for Kryo ser/de and the stack trace would look something like:
```
java.lang.UnsupportedOperationException
Serialization trace:
allProperties (org.apache.iceberg.aws.AwsClientProperties)
com.esotericsoftware.kryo.KryoException:
java.lang.UnsupportedOperationException
Serialization trace:
allProperties (org.apache.iceberg.aws.AwsClientProperties)
at
com.esotericsoftware.kryo.serializers.ObjectField.read(ObjectField.java:144)
at
com.esotericsoftware.kryo.serializers.FieldSerializer.read(FieldSerializer.java:543)
at com.esotericsoftware.kryo.Kryo.readClassAndObject(Kryo.java:813)
at
org.apache.iceberg.TestHelpers$KryoHelpers.roundTripSerialize(TestHelpers.java:292)
at
org.apache.iceberg.aws.AwsClientPropertiesTest.testKryoSerialization(AwsClientPropertiesTest.java:49)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
...
at
org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
at
worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
at
worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: java.lang.UnsupportedOperationException
at
org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap.put(ImmutableMap.java:814)
at
com.esotericsoftware.kryo.serializers.MapSerializer.read(MapSerializer.java:162)
at
com.esotericsoftware.kryo.serializers.MapSerializer.read(MapSerializer.java:39)
at com.esotericsoftware.kryo.Kryo.readObject(Kryo.java:731)
at
com.esotericsoftware.kryo.serializers.ObjectField.read(ObjectField.java:125)
... 88 more
```
The reason we use `SerializableMap` for Kryo is because Kryo can't deal with
Immutable collections and in most other places we pass around an
`ImmutableMap`. So in order to make `AwsClientProperties` work with Kryo, we
keep the properties in a `SerializableMap`.
In your case the issue seems to be due to
`java.lang.IndexOutOfBoundsException: Index 113 out of bounds for length 28`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
