-
-
Notifications
You must be signed in to change notification settings - Fork 362
Switch Pub/Sub implementation to use high level API #1700
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| using Google.Cloud.PubSub.V1; | ||
| using Google.Protobuf.Collections; | ||
| using Grpc.Core; | ||
| using JasperFx.Blocks; | ||
| using JasperFx.Core; | ||
|
|
@@ -53,72 +52,6 @@ IWolverineRuntime runtime | |
|
|
||
| NativeDeadLetterQueueEnabled = true; | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed this code block because dead lettering is handled by the |
||
| _acknowledge = new RetryBlock<string[]>(async (ackIds, _) => | ||
| { | ||
| if (transport.SubscriberApiClient is null) | ||
| { | ||
| throw new WolverinePubsubTransportNotConnectedException(); | ||
| } | ||
|
|
||
| if (ackIds.Any()) | ||
| { | ||
| await transport.SubscriberApiClient.AcknowledgeAsync( | ||
| _endpoint.Server.Subscription.Name, | ||
| ackIds | ||
| ); | ||
| } | ||
| }, _logger, runtime.Cancellation); | ||
|
|
||
| _deadLetter = new RetryBlock<Envelope>(async (e, _) => | ||
| { | ||
| if (_deadLetterTopic is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (e is PubsubEnvelope pubsubEnvelope) | ||
| { | ||
| await _acknowledge.PostAsync([pubsubEnvelope.AckId]); | ||
| } | ||
|
|
||
| await _deadLetterTopic.SendMessageAsync(e, _logger); | ||
| }, _logger, runtime.Cancellation); | ||
|
Comment on lines
-73
to
-86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Possibly here, new DLQ setup has to be written somewhere? |
||
|
|
||
| _requeue = new RetryBlock<Envelope>(async (e, _) => | ||
| { | ||
| if (e is PubsubEnvelope pubsubEnvelope) | ||
| { | ||
| await _acknowledge.PostAsync([pubsubEnvelope.AckId]); | ||
| } | ||
|
|
||
| await _endpoint.SendMessageAsync(e, _logger); | ||
| }, _logger, runtime.Cancellation); | ||
|
|
||
| _complete = new RetryBlock<Envelope[]>(async (envelopes, _) => | ||
| { | ||
| var pubsubEnvelopes = envelopes.OfType<PubsubEnvelope>().ToArray(); | ||
|
|
||
| if (!pubsubEnvelopes.Any()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (transport.SubscriberApiClient is null) | ||
| { | ||
| throw new WolverinePubsubTransportNotConnectedException(); | ||
| } | ||
|
|
||
| var ackIds = pubsubEnvelopes | ||
| .Select(e => e.AckId) | ||
| .Where(x => !string.IsNullOrEmpty(x)) | ||
| .Distinct() | ||
| .ToArray(); | ||
|
|
||
| await _acknowledge.PostAsync(ackIds); | ||
| }, _logger, _cancellation.Token); | ||
|
|
||
| _task = StartAsync(); | ||
| } | ||
|
|
||
| public Uri Address => _endpoint.Uri; | ||
|
|
@@ -232,61 +165,55 @@ protected async Task listenForMessagesAsync(Func<Task> listenAsync) | |
| } | ||
| } | ||
|
|
||
| protected async Task handleMessagesAsync(RepeatedField<ReceivedMessage> messages) | ||
| protected async Task handleMessagesAsync(PubsubMessage message) | ||
| { | ||
| var envelopes = new List<PubsubEnvelope>(messages.Count); | ||
| PubsubEnvelope? envelope = null; | ||
|
|
||
| foreach (var message in messages) | ||
| if (message.Attributes.ContainsKey("batched")) | ||
| { | ||
| if (message.Message.Attributes.Keys.Contains("batched")) | ||
| var batched = EnvelopeSerializer.ReadMany(message.Data.ToByteArray()); | ||
|
|
||
| if (batched.Any()) | ||
| { | ||
| var batched = EnvelopeSerializer.ReadMany(message.Message.Data.ToByteArray()); | ||
| await _receiver.ReceivedAsync(this, batched); | ||
| } | ||
|
|
||
| if (batched.Any()) | ||
| { | ||
| await _receiver.ReceivedAsync(this, batched); | ||
| } | ||
| return; | ||
| } | ||
|
Comment on lines
+172
to
+182
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not certain if this batching piece is still relevant with the higher level API |
||
|
|
||
| await _acknowledge.PostAsync([message.AckId]); | ||
| try | ||
| { | ||
| envelope = new PubsubEnvelope(); | ||
|
|
||
| continue; | ||
| } | ||
| _mapper.MapIncomingToEnvelope(envelope, message); | ||
|
|
||
| try | ||
| if (envelope.IsPing()) | ||
| { | ||
| var envelope = new PubsubEnvelope(); | ||
|
|
||
| _mapper.MapIncomingToEnvelope(envelope, message); | ||
|
|
||
| if (envelope.IsPing()) | ||
| try | ||
| { | ||
| try | ||
| { | ||
| await _complete.PostAsync([envelope]); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, | ||
| "{Uri}: Error while acknowledging Google Cloud Platform Pub/Sub ping message \"{AckId}\".", | ||
| _endpoint.Uri, message.AckId); | ||
| } | ||
|
|
||
| continue; | ||
| await _complete.PostAsync([envelope]); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, | ||
| "{Uri}: Error while acknowledging Google Cloud Platform Pub/Sub ping message \"{MessageId}\".", | ||
| _endpoint.Uri, message.MessageId); | ||
| } | ||
|
|
||
| envelopes.Add(envelope); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "{Uri}: Error while mapping Google Cloud Platform Pub/Sub message {AckId}.", | ||
| _endpoint.Uri, message.AckId); | ||
| return; | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "{Uri}: Error while mapping Google Cloud Platform Pub/Sub message {MessageId}.", | ||
| _endpoint.Uri, message.MessageId); | ||
| } | ||
|
|
||
|
|
||
| if (envelopes.Any()) | ||
| if (envelope != null) | ||
| { | ||
| await _receiver.ReceivedAsync(this, envelopes.ToArray()); | ||
| await _complete.PostAsync(envelopes.ToArray()); | ||
| await _receiver.ReceivedAsync(this, [envelope]); | ||
| await _complete.PostAsync([envelope]); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,4 +104,4 @@ public PubsubTopicListenerConfiguration ConfigureDeadLettering( | |
|
|
||
| return this; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acking should technically happen automatically here in this lambda on the return of
SubscriberClient.Reply.Ackso it seems like this removes some of the code that was previously managing the Ack at a lower level, but not clear if that code I removed is "structurally required" by Wolverine.