spring-attic / spring-cloud-gcp

Integration for Google Cloud Platform APIs with Spring
Apache License 2.0
705 stars 693 forks source link

PubSub issues - pull DeadlineExceededException, health indicator #2369

Open saturnism opened 4 years ago

saturnism commented 4 years ago

Describe the solution you'd like pubSubTemplate.pull(...) doesn't accept a timeout parameter. It defaults to ~5s, and throws DeadlineExceededException

  1. if pull fails, it should prob return null
  2. the timeout should be configurable
  3. health indicator should have conditional on `PubSubTemplate?

Additional context In Spring Integration, when using PubSubMessourceSource (pulled), it logs DeadlineExceededException every ~5s.

saturnism commented 4 years ago

https://github.com/googleapis/java-pubsub/issues/189

elefeint commented 4 years ago
  1. Health indicator autoconfig is conditional on PubSubTemplate bean, which would not be available without the enabled flag (or without being explicitly defined in user config).

Everything else is valid, though; DeadlineExceeded is a fairly normal scenario and we should not treat it as exceptional.

elefeint commented 4 years ago

gRPC timeout should be configurable through spring.cloud.gcp.pubsub.subscriber.retry.initial-rpc-timeout-seconds and spring.cloud.gcp.pubsub.subscriber.retry.total-timeout-seconds

elefeint commented 4 years ago

@saturnism, you were creating a message source with blockOnPull=true? (the alternative has been deprecated in API anyway, but just to confirm).

saturnism commented 4 years ago
  1. Health indicator autoconfig is conditional on PubSubTemplate bean, which would not be available without the enabled flag (or without being explicitly defined in user config).

I think we are missing the 'ConditionalOnBean` part. will update the comment.

saturnism commented 4 years ago

@saturnism, you were creating a message source with blockOnPull=true? (the alternative has been deprecated in API anyway, but just to confirm)

yes - blockOnPull=true

saturnism commented 4 years ago

thinking more about this.. if healthindicator is expected to autoconfigured when pubsub is enabled it prob shouldn't check on conditionalonbean, but check for whether pubsub starter is enabled or not?

elefeint commented 4 years ago

That's probably true. The only way for .enabled to be false is if someone explicitly disabled it, so it's a better indicator than presence of the bean.