Google-cloud-ruby: [BREAKING PRODUCTION] Error when attempting to add a retry policy to a pubsub subscription

[ianks]

Environment details

• OS: Mac and Linux
• Ruby version: 2.7.1
• Gem name and version: google-cloud-pubsub v2.3.0

Steps to reproduce

Attempting to set a retry policy on a subscription that already exists is now breaking. It used to work, but now errors out causing for our production application not to boot:

subscription.retry_policy = Google::Cloud::PubSub::RetryPolicy.new(
  minimum_backoff: CloudPubsub.config.auto_retry.minimum_backoff,
  maximum_backoff: CloudPubsub.config.auto_retry.maximum_backoff
)

And here is our pre-existing subscription:

<Google::Cloud::PubSub::V1::Subscription: name: "projects/adhawk-franchises-development/subscriptions/development.retailer.ctm_answer_call", topic: "", push_config: nil, ack_deadline_seconds: 0, retain_acked_messages: false, message_retention_duration: nil, labels: {}, enable_message_ordering: false, expiration_policy: nil, filter: "", dead_letter_policy: nil, retry_policy: <Google::Cloud::PubSub::V1::RetryPolicy: minimum_backoff: <Google::Protobuf::Duration: seconds: 30, nanos: 0>, maximum_backoff: <Google::Protobuf::Duration: seconds: 600, nanos: 0>>, detached: false>

Full backtrace

Google::Cloud::InvalidArgumentError: 3:Invalid resource name given (name=). Refer to https://cloud.google.com/pubsub/docs/admin#resource_names for more information.. debug_error_string:{"created":"@1607295488.846134000","description":"Error received from peer ipv4:142.250.73.202:443","file":"src/core/lib/surface/call.cc","file_line":1063,"grpc_message":"Invalid resource name given (name=). Refer to https://cloud.google.com/pubsub/docs/admin#resource_names for more information.","grpc_status":3}
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/google-cloud-pubsub-v1-0.1.2/lib/google/cloud/pubsub/v1/subscriber/client.rb:578:in `rescue in update_subscription'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/google-cloud-pubsub-v1-0.1.2/lib/google/cloud/pubsub/v1/subscriber/client.rb:544:in `update_subscription'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/google-cloud-pubsub-2.3.0/lib/google/cloud/pubsub/service.rb:194:in `update_subscription'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/google-cloud-pubsub-2.3.0/lib/google/cloud/pubsub/subscription.rb:594:in `retry_policy='
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/lib/hanami/events/cloud_pubsub/listener.rb:140:in `apply_retry_options'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/lib/hanami/events/cloud_pubsub/listener.rb:43:in `register'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/lib/hanami/events/adapter/cloud_pubsub.rb:91:in `register_listener'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/lib/hanami/events/adapter/cloud_pubsub.rb:67:in `subscribe'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/bundler/gems/events-59a36d22b75e/lib/hanami/events/base.rb:36:in `subscribe'
  /Users/ianks/Code/tatami/lib/subscribers/base.rb:29:in `setup_event_handler'
  /Users/ianks/Code/tatami/lib/subscribers/base.rb:23:in `subscribe_to'
  /Users/ianks/Code/tatami/apps/retailer/subscribers/call_tracking_metrics/answer_call.rb:10:in `<class:AnswerCall>'
  /Users/ianks/Code/tatami/apps/retailer/subscribers/call_tracking_metrics/answer_call.rb:9:in `<module:CallTrackingMetrics>'
  /Users/ianks/Code/tatami/apps/retailer/subscribers/call_tracking_metrics/answer_call.rb:8:in `<module:Subscribers>'
  /Users/ianks/Code/tatami/apps/retailer/subscribers/call_tracking_metrics/answer_call.rb:7:in `<module:Retailer>'
  /Users/ianks/Code/tatami/apps/retailer/subscribers/call_tracking_metrics/answer_call.rb:6:in `<main>'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `require'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:23:in `block in require_with_bootsnap_lfi'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/loaded_features_index.rb:92:in `register'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:22:in `require_with_bootsnap_lfi'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:31:in `require'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/bootsnap-1.5.1/lib/bootsnap/load_path_cache/core_ext/kernel_require.rb:53:in `require_relative'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-utils-1.3.6/lib/hanami/utils.rb:54:in `block in require!'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-utils-1.3.6/lib/hanami/utils.rb:92:in `each'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-utils-1.3.6/lib/hanami/utils.rb:92:in `for_each_file_in'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-utils-1.3.6/lib/hanami/utils.rb:54:in `require!'
  /Users/ianks/Code/tatami/config/environment.rb:116:in `block (3 levels) in <top (required)>'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/lib/hanami/events/cloud_pubsub/cli.rb:49:in `load_subscriptions'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/lib/hanami/events/cloud_pubsub/cli.rb:39:in `call'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-cli-0.3.1/lib/hanami/cli.rb:57:in `call'
  /Users/ianks/.asdf/installs/ruby/2.7.1/lib/ruby/gems/2.7.0/gems/hanami-events-cloud_pubsub-3.0.3/exe/cloudpubsub:8:in `<top (required)>'
  /Users/ianks/.asdf/installs/ruby/2.7.1/bin/cloudpubsub:23:in `load'
  /Users/ianks/.asdf/installs/ruby/2.7.1/bin/cloudpubsub:23:in `<top (required)>'

Making sure to follow these steps will guarantee the quickest resolution possible.

Thanks!

[ianks]

Here is exactly where the issue happens: https://github.com/broadlume/hanami-events-cloud_pubsub/blob/b4cbbee7567d1201af767725630995ea2e87e973/lib/hanami/events/cloud_pubsub/listener.rb#L135

[quartzmo]

@ianks Thank you for reporting this! I'll try to reproduce the error tomorrow.

[quartzmo]

@ianks I have been unable to reproduce this issue. I even modified the existing acceptance test for updating retry policy to use the subscription name you provided above (development.retailer.ctm_answer_call) and fetch the existing subscription before calling retry_policy=, but it still passes without errors. The Subscription resource name you are using looks good to me (rules below copied from API docs). I am unable to see why you get the error 3:Invalid resource name given (name=).

subscription_name (String) — Name of the new subscription. Must start with a letter, and contain only letters ([A-Za-z]), numbers ([0-9], dashes (-), underscores (_), periods (.), tildes (~), plus (+) or percent signs (%). It must be between 3 and 255 characters in length, and it must not start with "goog". Required.

[ianks]

Yeah it does not seem like anything should be wrong. In fact, it used to work until 12/4/2020 ish. One thing to note is that the subscription already has a retry policy... did you try setting it twice?

[quartzmo]

It used to work, but now errors out

Was the change related to updating the client libraries? If so, do you know which versions of google-cloud-pubsub and google-cloud-pubsub-v1 used to work? As you can see in the backtrace you provided, there is not a whole lot of client library code involved in this RPC call. The most recent change to this call stack that I can find was the introduction of google-cloud-pubsub-v1 around 4 months ago.

[quartzmo]

Sorry, in my comment above I forgot to set the link to the acceptance test for updating retry policy.

One thing to note is that the subscription already has a retry policy... did you try setting it twice?

The test provides an initial retry policy and then calls retry_policy= 3 more times. I'm happy to make some changes/additions to it if you can suggest any. I already tried a few myself locally. (I need to update this test in any case to remove the comment about the special naming prefix testdetachsubsxyz, which is no longer needed.)

[quartzmo]

If you want, you can try to run the test linked above with your own project and change it to best mimic your production context:

1. Clone the google-cloud-ruby GitHub repo.
2. Set environment variables with your project ID and service account credentials.
3. Follow the Setup instructions to install dependencies for google-cloud-ruby/google-cloud-pubsub.
4. Add focus to the blank line above the test (above the call to it). This will run just the one test.
5. Run: bundle exec rake acceptance. Confirm that the unmodified test passes using your project.
6. If the unmodified test passes with your project, modify the test to match your context, and re-run. Instead of letting the test create the subscription, you could try using an existing subscription known to cause the issue.
7. When you can demonstrate the problem with the service or client with a failing test, post your test here, or use a fork of the repo and a branch to publish it. Then I'll try to reproduce with my own project.

[ianks]

@quartzmo I was able to recreate this in the test suite using the same project, and the subscriptions / topics from our dev project:

    it "allows updating the retry policy of a found subscription (issue #8237)" do
      top = pubsub.find_topic("projects/adhawk-franchises-development/topics/development.ctm.answer_call")
      subscription = top.find_subscription "development.retailer.ctm_answer_call"
      subscription.retry_policy = Google::Cloud::PubSub::RetryPolicy.new
      _(subscription.retry_policy.minimum_backoff).must_equal 10 # Default value
      _(subscription.retry_policy.maximum_backoff).must_equal 600 # Default value

      subscription.retry_policy = Google::Cloud::PubSub::RetryPolicy.new minimum_backoff: retry_minimum_backoff
      _(subscription.retry_policy.minimum_backoff).must_equal retry_minimum_backoff
      _(subscription.retry_policy.maximum_backoff).must_equal 600 # Default value


      subscription.retry_policy = Google::Cloud::PubSub::RetryPolicy.new maximum_backoff: retry_maximum_backoff
      _(subscription.retry_policy.minimum_backoff).must_equal 10 # Default value
      _(subscription.retry_policy.maximum_backoff).must_equal retry_maximum_backoff
    end

Finished in 10.391834s, 0.0962 runs/s, 0.0962 assertions/s.

  1) Error:
Google::Cloud::PubSub::pubsub::Subscription on Topic#test_0002_allows updating the retry policy of a found subscription (issue #8237):
Google::Cloud::InvalidArgumentError: 3:Invalid resource name given (name=). Refer to https://cloud.google.com/pubsub/docs/admin#resource_names for more information.. debug_error_string:{"created":"@1607395008.698892000","description":"Error received from peer ipv4:142.250.73.202:443","file":"src/core/lib/surface/call.cc","file_line":1063,"grpc_message":"Invalid resource name given (name=). Refer to https://cloud.google.com/pubsub/docs/admin#resource_names for more information.","grpc_status":3}
    /Users/ianks/Code/google-cloud-ruby/google-cloud-pubsub-v1/lib/google/cloud/pubsub/v1/subscriber/client.rb:578:in `rescue in update_subscription'
    /Users/ianks/Code/google-cloud-ruby/google-cloud-pubsub-v1/lib/google/cloud/pubsub/v1/subscriber/client.rb:544:in `update_subscription'
    /Users/ianks/Code/google-cloud-ruby/google-cloud-pubsub/lib/google/cloud/pubsub/service.rb:194:in `update_subscription'
    /Users/ianks/Code/google-cloud-ruby/google-cloud-pubsub/lib/google/cloud/pubsub/subscription.rb:594:in `retry_policy='
    /Users/ianks/Code/google-cloud-ruby/google-cloud-pubsub/acceptance/pubsub/pubsub_test.rb:151:in `block (3 levels) in <top (required)>'

[ianks]

Ok so it definitely has to do with subscription that has already had a rtretry policy set. I verified this by:

1. Creating a new topic
2. Creating a new subscription to the topic
3. Setting the retry policy on the subscription (it works)
4. Finding the topic and subscription again
5. Setting the retry policy on the subscription to the same policy (it fails)

[quartzmo]

This is good news. I'm still unable to reproduce this locally, but it seems likely that the bug is in the value returned by Subscription#name. It appears to be nil. I'll keep trying later today.

[quartzmo]

@ianks I still haven't been able to reproduce your issue, but I did find an anomaly in Subscription#retry_policy=. Unlike the other methods that update attributes in Subscription, it conditionally retrieves the full resource representation before update if this is missing in the instance (due to the instance being created with skip_lookup: true as shown below):

pubsub = Google::Cloud::PubSub.new

# No API call is made to retrieve the subscription information.
sub = pubsub.subscription "my-sub", skip_lookup: true
sub.name #=> "projects/my-project/subscriptions/my-sub"

Reloading the resource should be a safe operation, and I can't see how it would result in a nil value for the Subscription name in the update request. However, I fixed it in unmerged PR #8374 to match the other methods. If you can easily try my branch in your environment, you might as well.

In your stepset above, steps 1-2 create the topic and subscription. Can you add these steps to the test you posted and confirm that it still reproduces the issue, so that I have a complete end-to-end repro that I can run?

[quartzmo]

@ianks Any updates?

[ORabbit]

The Cloud Pub/Sub dev team is aware of this issue and is working on resolving it.
I have reproduced the issue by doing the following:

1. gcloud pubsub topics create a-generic-topic
2. gcloud pubsub topics create a-dlq-topic
3. gcloud pubsub subscriptions create a-generic-sub-with-dlq --topic=a-generic-topic --dead-letter-topic=a-dlq-topic
4. gcloud pubsub subscriptions update a-generic-sub-with-dlq --max-retry-delay=30s
   
   
   
   • ERROR: (gcloud.pubsub.subscriptions.update) INVALID_ARGUMENT: Invalid resource name given (name=). Refer to https://cloud.google.com/pubsub/docs/admin#resource_names for more information.
   
   

The issue (at least the one reproduced above) stems from subscriptions with a DeadLetterPolicy enabled. This is not a direct issue with the client library, but an issue with the our servers.

In the meantime one mitigation option is to set the DeadLetterPolicy during an update to the already configured value. For example if we replace step 4 above with:

• gcloud pubsub subscriptions update a-generic-sub-with-dlq --max-retry-delay=30s --dead-letter-topic=a-dlq-topic

We will then succeed the request.

This can also be mitigated through the UI. I have not found a way to mitigate this in the Ruby client library yet.

[quartzmo]

I have not found a way to mitigate this in the Ruby client library yet.

@ORabbit What do you think about adding documentation of the issue to the client?

[ORabbit]

@quartzmo sorry for missing this post. Yes I believe adding it to the documentation would have been the right decision.

However, the fix is now live so the mitigation technique is no longer necessary.

@ianks could you confirm you are no longer impacted?

[quartzmo]

@ianks Is this issue resolved for you?

[quartzmo]

Closing due to lack of response. Please re-open if this is still a problem!