Skip to content

Commit 8f88065

Browse files
committed
Fix PulsarIO
The main issue for current PulsarIO.read is * It is based on Pulsar Reader instead of PulsarConsumer, which then do not support acknowledgement * The while() block in reader DoFn would never return until topic termination, this basically means pipeline stuck * The restriction is on publishTime, and tryClaim assumes its ordering. This is not true. reader returning message is ordered on messageId. This is a wrong choice. Currently unresolved * PulsarMessage's coder implementation dropped message. This causes Data loss if the PulsarIO.read do not follow an immediate mapping * Tests are defunct and errors are suppressed, making them succeed spuriously Current PulsarIO.write is even more primitive. Pipeline expansion actually fails. It is not idempotent. Major fixes include * Allow Pulsar reader to have a timeout * Fix PulsarMessage and coder to include serializable fields from message * Fix mock client/reader and add a full read pipeline in test * Fix issues prevent PulsarIO.write from expanding. now it works minimally, that is publish every message received (at least once). * Working integration tests for read and write This has made PulsarIO.read minimally functionable. Although it won't split and can only run single thread. Going forward, we should re-implement reader DoFn based on Pulsar consumer. Thoughs rename the current DoFn to "NaiveReadFromPulsarDoFn"
1 parent 9651c64 commit 8f88065

17 files changed

Lines changed: 741 additions & 534 deletions

.github/workflows/beam_PreCommit_Java_Pulsar_IO_Direct.yml

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,31 +21,12 @@ on:
2121
branches: ['master', 'release-*']
2222
paths:
2323
- "sdks/java/io/pulsar/**"
24-
- "sdks/java/io/common/**"
25-
- "sdks/java/core/src/main/**"
26-
- "build.gradle"
27-
- "buildSrc/**"
28-
- "gradle/**"
29-
- "gradle.properties"
30-
- "gradlew"
31-
- "gradle.bat"
32-
- "settings.gradle.kts"
3324
- ".github/workflows/beam_PreCommit_Java_Pulsar_IO_Direct.yml"
3425
pull_request_target:
3526
branches: ['master', 'release-*']
3627
paths:
3728
- "sdks/java/io/pulsar/**"
38-
- "sdks/java/io/common/**"
39-
- "sdks/java/core/src/main/**"
40-
- 'release/trigger_all_tests.json'
41-
- '.github/trigger_files/beam_PreCommit_Java_Pulsar_IO_Direct.json'
42-
- "build.gradle"
43-
- "buildSrc/**"
44-
- "gradle/**"
45-
- "gradle.properties"
46-
- "gradlew"
47-
- "gradle.bat"
48-
- "settings.gradle.kts"
29+
- ".github/workflows/beam_PreCommit_Java_Pulsar_IO_Direct.yml"
4930
issue_comment:
5031
types: [created]
5132
schedule:
@@ -110,6 +91,13 @@ jobs:
11091
arguments: |
11192
-PdisableSpotlessCheck=true \
11293
-PdisableCheckStyle=true \
94+
- name: run Pulsar IO IT script
95+
uses: ./.github/actions/gradle-command-self-hosted-action
96+
with:
97+
gradle-command: :sdks:java:io:pulsar:integrationTest
98+
arguments: |
99+
-PdisableSpotlessCheck=true \
100+
-PdisableCheckStyle=true \
113101
- name: Archive JUnit Test Results
114102
uses: actions/upload-artifact@v4
115103
if: ${{ !success() }}
@@ -135,4 +123,4 @@ jobs:
135123
if: always()
136124
with:
137125
name: Publish SpotBugs
138-
path: '**/build/reports/spotbugs/*.html'
126+
path: '**/build/reports/spotbugs/*.html'

build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,7 @@ tasks.register("javaioPreCommit") {
355355
dependsOn(":sdks:java:io:mqtt:build")
356356
dependsOn(":sdks:java:io:neo4j:build")
357357
dependsOn(":sdks:java:io:parquet:build")
358+
dependsOn(":sdks:java:io:pulsar:build")
358359
dependsOn(":sdks:java:io:rabbitmq:build")
359360
dependsOn(":sdks:java:io:redis:build")
360361
dependsOn(":sdks:java:io:rrio:build")

sdks/java/io/pulsar/build.gradle

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,31 +18,32 @@
1818

1919
plugins { id 'org.apache.beam.module' }
2020
applyJavaNature(automaticModuleName: 'org.apache.beam.sdk.io.pulsar')
21+
enableJavaPerformanceTesting()
2122

2223
description = "Apache Beam :: SDKs :: Java :: IO :: Pulsar"
2324
ext.summary = "IO to read and write to Pulsar"
2425

25-
def pulsar_version = '2.8.2'
26+
def pulsar_version = '2.11.4'
2627

2728

2829
dependencies {
2930
implementation library.java.vendored_guava_32_1_2_jre
3031
implementation library.java.slf4j_api
3132
implementation library.java.joda_time
3233

33-
implementation "org.apache.pulsar:pulsar-client:$pulsar_version"
34-
implementation "org.apache.pulsar:pulsar-client-admin:$pulsar_version"
35-
permitUnusedDeclared "org.apache.pulsar:pulsar-client:$pulsar_version"
36-
permitUnusedDeclared "org.apache.pulsar:pulsar-client-admin:$pulsar_version"
37-
permitUsedUndeclared "org.apache.pulsar:pulsar-client-api:$pulsar_version"
38-
permitUsedUndeclared "org.apache.pulsar:pulsar-client-admin-api:$pulsar_version"
34+
implementation "org.apache.pulsar:pulsar-client-api:$pulsar_version"
35+
implementation "org.apache.pulsar:pulsar-client-admin-api:$pulsar_version"
36+
runtimeOnly "org.apache.pulsar:pulsar-client:$pulsar_version"
37+
runtimeOnly("org.apache.pulsar:pulsar-client-admin:$pulsar_version") {
38+
// To prevent a StackOverflow within Pulsar admin client because JUL -> SLF4J -> JUL
39+
exclude group: "org.slf4j", module: "jul-to-slf4j"
40+
}
3941

4042
implementation project(path: ":sdks:java:core", configuration: "shadow")
4143

42-
testImplementation library.java.jupiter_api
43-
testRuntimeOnly library.java.jupiter_engine
44+
testImplementation library.java.junit
45+
testRuntimeOnly library.java.slf4j_jdk14
4446
testRuntimeOnly project(path: ":runners:direct-java", configuration: "shadow")
4547
testImplementation "org.testcontainers:pulsar:1.15.3"
4648
testImplementation "org.assertj:assertj-core:2.9.1"
47-
4849
}

sdks/java/io/pulsar/src/main/java/org/apache/beam/sdk/io/pulsar/ReadFromPulsarDoFn.java renamed to sdks/java/io/pulsar/src/main/java/org/apache/beam/sdk/io/pulsar/NaiveReadFromPulsarDoFn.java

Lines changed: 110 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@
1717
*/
1818
package org.apache.beam.sdk.io.pulsar;
1919

20-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2120
import java.io.IOException;
21+
import java.time.Duration;
2222
import java.util.concurrent.TimeUnit;
2323
import org.apache.beam.sdk.coders.Coder;
2424
import org.apache.beam.sdk.io.range.OffsetRange;
25+
import org.apache.beam.sdk.options.PipelineOptions;
2526
import org.apache.beam.sdk.transforms.DoFn;
2627
import org.apache.beam.sdk.transforms.SerializableFunction;
2728
import org.apache.beam.sdk.transforms.splittabledofn.GrowableOffsetRangeTracker;
@@ -30,6 +31,9 @@
3031
import org.apache.beam.sdk.transforms.splittabledofn.WatermarkEstimator;
3132
import org.apache.beam.sdk.transforms.splittabledofn.WatermarkEstimators;
3233
import org.apache.beam.sdk.transforms.windowing.BoundedWindow;
34+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.MoreObjects;
35+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Stopwatch;
36+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Strings;
3337
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Supplier;
3438
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.Suppliers;
3539
import org.apache.pulsar.client.admin.PulsarAdmin;
@@ -40,68 +44,73 @@
4044
import org.apache.pulsar.client.api.PulsarClientException;
4145
import org.apache.pulsar.client.api.Reader;
4246
import org.apache.pulsar.client.api.ReaderBuilder;
47+
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
48+
import org.checkerframework.checker.nullness.qual.Nullable;
4349
import org.joda.time.Instant;
4450
import org.slf4j.Logger;
4551
import org.slf4j.LoggerFactory;
4652

4753
/**
48-
* Transform for reading from Apache Pulsar. Support is currently incomplete, and there may be bugs;
49-
* see https://github.com/apache/beam/issues/31078 for more info, and comment in that issue if you
50-
* run into issues with this IO.
54+
* DoFn for reading from Apache Pulsar based on Pulsar {@link Reader} from the start message id. It
55+
* does not support split or acknowledge message get read.
5156
*/
5257
@DoFn.UnboundedPerElement
53-
@SuppressWarnings({"rawtypes", "nullness"})
54-
@SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "Initialization is safe.")
55-
public class ReadFromPulsarDoFn extends DoFn<PulsarSourceDescriptor, PulsarMessage> {
58+
@SuppressWarnings("nullness")
59+
public class NaiveReadFromPulsarDoFn<T> extends DoFn<PulsarSourceDescriptor, T> {
5660

57-
private static final Logger LOG = LoggerFactory.getLogger(ReadFromPulsarDoFn.class);
58-
private SerializableFunction<String, PulsarClient> pulsarClientSerializableFunction;
59-
private PulsarClient client;
60-
private PulsarAdmin admin;
61-
private String clientUrl;
62-
private String adminUrl;
61+
private static final Logger LOG = LoggerFactory.getLogger(NaiveReadFromPulsarDoFn.class);
62+
private final SerializableFunction<String, PulsarClient> clientFn;
63+
private final SerializableFunction<String, PulsarAdmin> adminFn;
64+
private final SerializableFunction<Message<?>, T> outputFn;
65+
private final java.time.Duration pollingTimeout;
66+
private transient @MonotonicNonNull PulsarClient client;
67+
private transient @MonotonicNonNull PulsarAdmin admin;
68+
private @MonotonicNonNull String clientUrl;
69+
private @Nullable final String adminUrl;
6370

6471
private final SerializableFunction<Message<byte[]>, Instant> extractOutputTimestampFn;
6572

66-
public ReadFromPulsarDoFn(PulsarIO.Read transform) {
67-
this.extractOutputTimestampFn = transform.getExtractOutputTimestampFn();
73+
public NaiveReadFromPulsarDoFn(PulsarIO.Read<T> transform) {
74+
this.extractOutputTimestampFn =
75+
transform.getTimestampType() == PulsarIO.ReadTimestampType.PUBLISH_TIME
76+
? record -> new Instant(record.getPublishTime())
77+
: ignored -> Instant.now();
78+
this.pollingTimeout = Duration.ofSeconds(transform.getConsumerPollingTimeout());
79+
this.outputFn = transform.getOutputFn();
6880
this.clientUrl = transform.getClientUrl();
6981
this.adminUrl = transform.getAdminUrl();
70-
this.pulsarClientSerializableFunction = transform.getPulsarClient();
82+
this.clientFn =
83+
MoreObjects.firstNonNull(
84+
transform.getPulsarClient(), PulsarIOUtils.PULSAR_CLIENT_SERIALIZABLE_FUNCTION);
85+
this.adminFn =
86+
MoreObjects.firstNonNull(
87+
transform.getPulsarAdmin(), PulsarIOUtils.PULSAR_ADMIN_SERIALIZABLE_FUNCTION);
88+
admin = null;
7189
}
7290

73-
// Open connection to Pulsar clients
91+
/** Open connection to Pulsar clients. */
7492
@Setup
7593
public void initPulsarClients() throws Exception {
76-
if (this.clientUrl == null) {
77-
this.clientUrl = PulsarIOUtils.SERVICE_URL;
78-
}
79-
if (this.adminUrl == null) {
80-
this.adminUrl = PulsarIOUtils.SERVICE_HTTP_URL;
81-
}
82-
83-
if (this.client == null) {
84-
this.client = pulsarClientSerializableFunction.apply(this.clientUrl);
85-
if (this.client == null) {
86-
this.client = PulsarClient.builder().serviceUrl(clientUrl).build();
94+
if (client == null) {
95+
if (clientUrl == null) {
96+
clientUrl = PulsarIOUtils.LOCAL_SERVICE_URL;
8797
}
98+
client = clientFn.apply(clientUrl);
8899
}
89100

90-
if (this.admin == null) {
91-
this.admin =
92-
PulsarAdmin.builder()
93-
.serviceHttpUrl(adminUrl)
94-
.tlsTrustCertsFilePath(null)
95-
.allowTlsInsecureConnection(false)
96-
.build();
101+
// admin is optional
102+
if (this.admin == null && !Strings.isNullOrEmpty(adminUrl)) {
103+
admin = adminFn.apply(adminUrl);
97104
}
98105
}
99106

100-
// Close connection to Pulsar clients
107+
/** Close connection to Pulsar clients. */
101108
@Teardown
102109
public void teardown() throws Exception {
103110
this.client.close();
104-
this.admin.close();
111+
if (this.admin != null) {
112+
this.admin.close();
113+
}
105114
}
106115

107116
@GetInitialRestriction
@@ -152,25 +161,47 @@ public Coder<OffsetRange> getRestrictionCoder() {
152161
public ProcessContinuation processElement(
153162
@Element PulsarSourceDescriptor pulsarSourceDescriptor,
154163
RestrictionTracker<OffsetRange, Long> tracker,
155-
WatermarkEstimator watermarkEstimator,
156-
OutputReceiver<PulsarMessage> output)
164+
WatermarkEstimator<Instant> watermarkEstimator,
165+
OutputReceiver<T> output)
157166
throws IOException {
158167
long startTimestamp = tracker.currentRestriction().getFrom();
159168
String topicDescriptor = pulsarSourceDescriptor.getTopic();
160169
try (Reader<byte[]> reader = newReader(this.client, topicDescriptor)) {
161170
if (startTimestamp > 0) {
171+
// seek is inclusive
162172
reader.seek(startTimestamp);
163173
}
164-
while (true) {
165-
if (reader.hasReachedEndOfTopic()) {
166-
reader.close();
167-
return ProcessContinuation.stop();
174+
if (reader.hasReachedEndOfTopic()) {
175+
// topic has terminated
176+
tracker.tryClaim(Long.MAX_VALUE);
177+
reader.close();
178+
return ProcessContinuation.stop();
179+
}
180+
final Stopwatch pollTimer = Stopwatch.createUnstarted();
181+
Duration remainingTimeout = pollingTimeout;
182+
while (Duration.ZERO.compareTo(remainingTimeout) < 0) {
183+
pollTimer.reset().start();
184+
Message<byte[]> message =
185+
reader.readNext((int) remainingTimeout.toMillis(), TimeUnit.MILLISECONDS);
186+
final Duration elapsed = pollTimer.elapsed();
187+
try {
188+
remainingTimeout = remainingTimeout.minus(elapsed);
189+
} catch (ArithmeticException e) {
190+
remainingTimeout = Duration.ZERO;
168191
}
169-
Message<byte[]> message = reader.readNext();
192+
// No progress when the polling timeout expired.
193+
// Self-checkpoint and move to process the next element.
170194
if (message == null) {
171195
return ProcessContinuation.resume();
172196
}
173197
Long currentTimestamp = message.getPublishTime();
198+
if (currentTimestamp < startTimestamp) {
199+
LOG.warn(
200+
"Skip late message of publish time {} before startTimestamp {}",
201+
currentTimestamp,
202+
startTimestamp);
203+
continue;
204+
}
174205
// if tracker.tryclaim() return true, sdf must execute work otherwise
175206
// doFn must exit processElement() without doing any work associated
176207
// or claiming more work
@@ -186,12 +217,21 @@ public ProcessContinuation processElement(
186217
return ProcessContinuation.stop();
187218
}
188219
}
189-
PulsarMessage pulsarMessage =
190-
new PulsarMessage(message.getTopicName(), message.getPublishTime(), message);
220+
T messageT = outputFn.apply(message);
191221
Instant outputTimestamp = extractOutputTimestampFn.apply(message);
192-
output.outputWithTimestamp(pulsarMessage, outputTimestamp);
222+
output.outputWithTimestamp(messageT, outputTimestamp);
193223
}
194224
}
225+
return ProcessContinuation.resume();
226+
}
227+
228+
@SplitRestriction
229+
public void splitRestriction(
230+
@Restriction OffsetRange restriction,
231+
OutputReceiver<OffsetRange> receiver,
232+
PipelineOptions unused) {
233+
// read based on Reader does not support split
234+
receiver.output(restriction);
195235
}
196236

197237
@GetInitialWatermarkEstimatorState
@@ -221,27 +261,34 @@ public OffsetRangeTracker restrictionTracker(
221261
private static class PulsarLatestOffsetEstimator
222262
implements GrowableOffsetRangeTracker.RangeEndEstimator {
223263

224-
private final Supplier<Message> memoizedBacklog;
264+
private final @Nullable Supplier<Message<byte[]>> memoizedBacklog;
225265

226-
private PulsarLatestOffsetEstimator(PulsarAdmin admin, String topic) {
227-
this.memoizedBacklog =
228-
Suppliers.memoizeWithExpiration(
229-
() -> {
230-
try {
231-
Message<byte[]> lastMsg = admin.topics().examineMessage(topic, "latest", 1);
232-
return lastMsg;
233-
} catch (PulsarAdminException e) {
234-
throw new RuntimeException(e);
235-
}
236-
},
237-
1,
238-
TimeUnit.SECONDS);
266+
private PulsarLatestOffsetEstimator(@Nullable PulsarAdmin admin, String topic) {
267+
if (admin != null) {
268+
this.memoizedBacklog =
269+
Suppliers.memoizeWithExpiration(
270+
() -> {
271+
try {
272+
return admin.topics().examineMessage(topic, "latest", 1);
273+
} catch (PulsarAdminException e) {
274+
throw new RuntimeException(e);
275+
}
276+
},
277+
1,
278+
TimeUnit.SECONDS);
279+
} else {
280+
memoizedBacklog = null;
281+
}
239282
}
240283

241284
@Override
242285
public long estimate() {
243-
Message<byte[]> msg = memoizedBacklog.get();
244-
return msg.getPublishTime();
286+
if (memoizedBacklog != null) {
287+
Message<byte[]> msg = memoizedBacklog.get();
288+
return msg.getPublishTime();
289+
} else {
290+
return Long.MIN_VALUE;
291+
}
245292
}
246293
}
247294

0 commit comments

Comments
 (0)