Skip to content

Commit eb9e7a3

Browse files
authored
Override default fadvise to fix regression from gcs-connector v3 upgrade (#39445)
* Support serialization and deserialization for GoogleCloudStorageReadOptions Fixes an issue where GoogleCloudStorageReadOptions set on GcsOptions was annotated with @JsonIgnore, causing it to be omitted when serializing PipelineOptions for remote workers (e.g., Dataflow) and falling back to defaults. * Add a unit test * Override default fadvice to SEQUENTIAL This fixes the regression introduced by gcs-connector v3. * Spotless * Only support a subset of fields in GoogleCloudStorageReadOptions. * Refactor
1 parent 9c3f51e commit eb9e7a3

4 files changed

Lines changed: 133 additions & 2 deletions

File tree

sdks/java/extensions/google-cloud-platform-core/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ dependencies {
5656
implementation library.java.http_core
5757
implementation library.java.http_client
5858
implementation library.java.jackson_annotations
59+
implementation library.java.jackson_core
5960
implementation library.java.jackson_databind
6061
permitUnusedDeclared library.java.jackson_databind // BEAM-11761
6162
testImplementation project(path: ":sdks:java:core", configuration: "shadowTest")

sdks/java/extensions/google-cloud-platform-core/src/main/java/org/apache/beam/sdk/extensions/gcp/options/GcsOptions.java

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,15 @@
1818
package org.apache.beam.sdk.extensions.gcp.options;
1919

2020
import com.fasterxml.jackson.annotation.JsonIgnore;
21+
import com.fasterxml.jackson.core.JsonGenerator;
22+
import com.fasterxml.jackson.core.JsonParser;
23+
import com.fasterxml.jackson.databind.DeserializationContext;
24+
import com.fasterxml.jackson.databind.JsonDeserializer;
25+
import com.fasterxml.jackson.databind.JsonNode;
26+
import com.fasterxml.jackson.databind.JsonSerializer;
27+
import com.fasterxml.jackson.databind.SerializerProvider;
28+
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
29+
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
2130
import com.google.cloud.hadoop.gcsio.GoogleCloudStorageReadOptions;
2231
import com.google.cloud.hadoop.util.AsyncWriteChannelOptions;
2332
import java.util.HashMap;
@@ -49,12 +58,16 @@ public interface GcsOptions extends ApplicationNameOptions, GcpOptions, Pipeline
4958
class GcsReadOptionsFactory implements DefaultValueFactory<GoogleCloudStorageReadOptions> {
5059
@Override
5160
public GoogleCloudStorageReadOptions create(PipelineOptions options) {
52-
return GoogleCloudStorageReadOptions.DEFAULT;
61+
// In gcs-connector v3, GoogleCloudStorageReadOptions.DEFAULT changed fadvise from SEQUENTIAL
62+
// to AUTO. Beam workloads default to SEQUENTIAL to preserve expected sequential read
63+
// throughput and caching behavior.
64+
return GcsReadOptionsSerializer.DEFAULT_OPTIONS;
5365
}
5466
}
5567

5668
/** @deprecated This option will be removed in a future release. */
57-
@JsonIgnore
69+
@JsonSerialize(using = GcsReadOptionsSerializer.class)
70+
@JsonDeserialize(using = GcsReadOptionsDeserializer.class)
5871
@Description(
5972
"The GoogleCloudStorageReadOptions instance that should be used to read from Google Cloud Storage.")
6073
@Default.InstanceFactory(GcsReadOptionsFactory.class)
@@ -286,3 +299,71 @@ boolean exceedsEntryLimit() {
286299
}
287300
}
288301
}
302+
303+
class GcsReadOptionsSerializer extends JsonSerializer<GoogleCloudStorageReadOptions> {
304+
static final GoogleCloudStorageReadOptions DEFAULT_OPTIONS =
305+
GoogleCloudStorageReadOptions.DEFAULT
306+
.toBuilder()
307+
.setFadvise(GoogleCloudStorageReadOptions.Fadvise.SEQUENTIAL)
308+
.build();
309+
310+
@Override
311+
public void serialize(
312+
GoogleCloudStorageReadOptions value, JsonGenerator gen, SerializerProvider serializers)
313+
throws java.io.IOException {
314+
// Note: We only support a partial set of options to propagate to remote
315+
// workers. Setting the unsupported ones will not have any effect and will fall
316+
// back to default in remote workers. Support for additional options can be
317+
// added on a need basis. The full list of options can be seen in
318+
// com.google.cloud.hadoop.gcsio.GoogleCloudStorageReadOptions.
319+
gen.writeStartObject();
320+
if (value.getFadvise() != null && value.getFadvise() != DEFAULT_OPTIONS.getFadvise()) {
321+
gen.writeStringField("fadvise", value.getFadvise().name());
322+
}
323+
if (value.isFastFailOnNotFoundEnabled() != DEFAULT_OPTIONS.isFastFailOnNotFoundEnabled()) {
324+
gen.writeBooleanField("fastFailOnNotFoundEnabled", value.isFastFailOnNotFoundEnabled());
325+
}
326+
if (value.getMinRangeRequestSize() != DEFAULT_OPTIONS.getMinRangeRequestSize()) {
327+
gen.writeNumberField("minRangeRequestSize", value.getMinRangeRequestSize());
328+
}
329+
if (value.getInplaceSeekLimit() != DEFAULT_OPTIONS.getInplaceSeekLimit()) {
330+
gen.writeNumberField("inplaceSeekLimit", value.getInplaceSeekLimit());
331+
}
332+
if (value.isGrpcReadZeroCopyEnabled() != DEFAULT_OPTIONS.isGrpcReadZeroCopyEnabled()) {
333+
gen.writeBooleanField("grpcReadZeroCopyEnabled", value.isGrpcReadZeroCopyEnabled());
334+
}
335+
gen.writeEndObject();
336+
}
337+
}
338+
339+
class GcsReadOptionsDeserializer extends JsonDeserializer<GoogleCloudStorageReadOptions> {
340+
@Override
341+
public GoogleCloudStorageReadOptions deserialize(JsonParser p, DeserializationContext ctxt)
342+
throws java.io.IOException {
343+
JsonNode root = p.readValueAsTree();
344+
GoogleCloudStorageReadOptions.Builder builder =
345+
GcsReadOptionsSerializer.DEFAULT_OPTIONS.toBuilder();
346+
347+
if (root != null && root.isObject()) {
348+
if (root.hasNonNull("fadvise")) {
349+
builder.setFadvise(
350+
GoogleCloudStorageReadOptions.Fadvise.valueOf(root.get("fadvise").asText()));
351+
}
352+
if (root.hasNonNull("fastFailOnNotFoundEnabled")) {
353+
builder.setFastFailOnNotFoundEnabled(root.get("fastFailOnNotFoundEnabled").asBoolean());
354+
} else if (root.hasNonNull("fastFailOnNotFound")) {
355+
builder.setFastFailOnNotFoundEnabled(root.get("fastFailOnNotFound").asBoolean());
356+
}
357+
if (root.hasNonNull("minRangeRequestSize")) {
358+
builder.setMinRangeRequestSize(root.get("minRangeRequestSize").asLong());
359+
}
360+
if (root.hasNonNull("inplaceSeekLimit")) {
361+
builder.setInplaceSeekLimit(root.get("inplaceSeekLimit").asLong());
362+
}
363+
if (root.hasNonNull("grpcReadZeroCopyEnabled")) {
364+
builder.setGrpcReadZeroCopyEnabled(root.get("grpcReadZeroCopyEnabled").asBoolean());
365+
}
366+
}
367+
return builder.build();
368+
}
369+
}

sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/GcpCoreApiSurfaceTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public void testGcpCoreApiSurface() throws Exception {
5151
final Set<Matcher<Class<?>>> allowedClasses =
5252
ImmutableSet.of(
5353
classesInPackage("com.fasterxml.jackson.annotation"),
54+
classesInPackage("com.fasterxml.jackson.core"),
55+
classesInPackage("com.fasterxml.jackson.databind"),
5456
classesInPackage("com.google.api.client.googleapis"),
5557
classesInPackage("com.google.api.client.http"),
5658
classesInPackage("com.google.api.client.json"),

sdks/java/extensions/google-cloud-platform-core/src/test/java/org/apache/beam/sdk/extensions/gcp/options/GcsOptionsTest.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,16 @@
1818
package org.apache.beam.sdk.extensions.gcp.options;
1919

2020
import static org.junit.Assert.assertEquals;
21+
import static org.junit.Assert.assertFalse;
22+
import static org.junit.Assert.assertNotNull;
2123
import static org.junit.Assert.assertThrows;
2224

25+
import com.fasterxml.jackson.databind.ObjectMapper;
26+
import com.google.cloud.hadoop.gcsio.GoogleCloudStorageReadOptions;
2327
import java.util.Collections;
2428
import java.util.HashMap;
2529
import java.util.Map;
30+
import org.apache.beam.sdk.options.PipelineOptions;
2631
import org.apache.beam.sdk.options.PipelineOptionsFactory;
2732
import org.junit.Test;
2833
import org.junit.runner.RunWith;
@@ -75,4 +80,46 @@ public void testEntriesWithErrors() throws Exception {
7580
IllegalArgumentException.class,
7681
() -> PipelineOptionsFactory.fromArgs(TOO_MANY_ENTRIES_WITH_JOB).as(GcsOptions.class));
7782
}
83+
84+
@Test
85+
public void testGoogleCloudStorageReadOptionsSerialization() throws Exception {
86+
GcsOptions options = PipelineOptionsFactory.as(GcsOptions.class);
87+
GoogleCloudStorageReadOptions readOptions =
88+
GoogleCloudStorageReadOptions.builder()
89+
.setFadvise(GoogleCloudStorageReadOptions.Fadvise.RANDOM)
90+
.setFastFailOnNotFoundEnabled(false)
91+
.setMinRangeRequestSize(12345L)
92+
.build();
93+
options.setGoogleCloudStorageReadOptions(readOptions);
94+
95+
ObjectMapper mapper = new ObjectMapper();
96+
String serialized = mapper.writeValueAsString(options);
97+
GcsOptions deserialized =
98+
mapper.readValue(serialized, PipelineOptions.class).as(GcsOptions.class);
99+
100+
GoogleCloudStorageReadOptions deserializedReadOptions =
101+
deserialized.getGoogleCloudStorageReadOptions();
102+
103+
assertNotNull(deserializedReadOptions);
104+
assertEquals(
105+
GoogleCloudStorageReadOptions.Fadvise.RANDOM, deserializedReadOptions.getFadvise());
106+
assertFalse(deserializedReadOptions.isFastFailOnNotFoundEnabled());
107+
assertEquals(12345L, deserializedReadOptions.getMinRangeRequestSize());
108+
}
109+
110+
@Test
111+
public void testDefaultGoogleCloudStorageReadOptionsSerialization() throws Exception {
112+
GcsOptions options = PipelineOptionsFactory.as(GcsOptions.class);
113+
ObjectMapper mapper = new ObjectMapper();
114+
String serialized = mapper.writeValueAsString(options);
115+
GcsOptions deserialized =
116+
mapper.readValue(serialized, PipelineOptions.class).as(GcsOptions.class);
117+
118+
GoogleCloudStorageReadOptions deserializedReadOptions =
119+
deserialized.getGoogleCloudStorageReadOptions();
120+
121+
assertNotNull(deserializedReadOptions);
122+
assertEquals(
123+
GoogleCloudStorageReadOptions.Fadvise.SEQUENTIAL, deserializedReadOptions.getFadvise());
124+
}
78125
}

0 commit comments

Comments
 (0)