PubSub: Default thread-based subscriber Policy's on_response() implementation degenerates into series of synchronous calls because of debug statement #4167

Activity

1. jeffkpayne

   changed the title [-]Default thread-based PubSub subscriber Policy's on_response() implementation degenerates into series of synchronous calls because of debug statement[/-] [+]PubSub: Default thread-based subscriber Policy's on_response() implementation degenerates into series of synchronous calls because of debug statement[/+] on Oct 12, 2017
2. theacodes commented on Oct 12, 2017

   

   theacodes

   on Oct 12, 2017 via email

   Contributor

   More actions

   You're totally right. That should be done via add_done_callback instead.

   …

   On Wed, Oct 11, 2017, 10:21 PM Jeff Payne ***@***.***> wrote: https://github.com/GoogleCloudPlatform/google-cloud-python/blob/4e17158c0f14e0e91cbeb74c3e8483c5a231e9ac/pubsub/google/cloud/pubsub_v1/subscriber/policy/thread.py#L147 While testing out the new PubSub API (v 0.28.3), I noticed that the default thread-based Policy, which uses a ThreadPoolExecutor(max_workers=10) seemed to be executing the subscriber callbacks sequentially, instead of asynchronously. Upon introspection of the source, it looks like the above referenced code: ... future = self._executor.submit(self._callback, message) logger.debug('Result: %s' % future.result()) will block until self._callback completes, because of the call to future.result(). Or am I losing my mind? Thanks! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4167>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUcyH9glRDri8lfWWL-oFringJqy4Xks5sraHRgaJpZM4P2exY> .
3. jeffkpayne

   added 2 commits that reference this issue on Oct 12, 2017

   googleapis#4167 Remove debug logging call

   b620dc8

   Move debug logging call in Future.add_done_callback (googleapis#4167)

   6d82f99
4. lukesneeringer commented on Oct 12, 2017

   

   lukesneeringer

   on Oct 12, 2017

   Contributor

   More actions

   I am noticing @jeffkpayne already fixing the problem, so I am just planning on taking that PR when it is provided. I will knock it out this afternoon if he does not get to it in time, since obviously this is pretty critical to fix.
5. lukesneeringer

   added

   api: pubsubIssues related to the Pub/Sub API.Issues related to the Pub/Sub API.

   priority: p1Important issue which blocks shipping the next release. Will be fixed prior to next release.Important issue which blocks shipping the next release. Will be fixed prior to next release.

   on Oct 12, 2017
6. lukesneeringer

   self-assigned this

   on Oct 12, 2017
7. jeffkpayne commented on Oct 12, 2017

   

   jeffkpayne

   on Oct 12, 2017

   ContributorAuthor

   More actions

   @lukesneeringer trying to get my local env setup to run the system tests, etc, as outlined in CONTRIBUTING.rst now...
8. lukesneeringer commented on Oct 12, 2017

   

   lukesneeringer

   on Oct 12, 2017

   Contributor

   More actions

   @jeffkpayne If you have unit tests working and coverage, feel free to build a PR and I will deal with system tests for you. :-)
9. jeffkpayne

   added a commit that references this issue on Oct 12, 2017

   Move debug logging call in Future.add_done_callback (googleapis#4167)

   4c86521
10. lukesneeringer

    closed this as completedin #4174on Oct 13, 2017