Skip to content
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

pubsub.ReceiveSettings.NumGoroutines description inaccurate? #712

Closed
zhenjl opened this issue Jul 30, 2017 · 5 comments
Closed

pubsub.ReceiveSettings.NumGoroutines description inaccurate? #712

zhenjl opened this issue Jul 30, 2017 · 5 comments
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API.
Milestone

Comments

@zhenjl
Copy link

zhenjl commented Jul 30, 2017

Hi, I ran into an issue these last few days with pubsub processing more messages than I expected. After digging into this a bit, it looks like I may have misinterpreted what this means. Is this a bug or intended behavior?

On the surface, after reading this description, I expected that if I set NumGoroutines to 1, my code will only receive 1 message at a time.

	// NumGoroutines is the number of goroutines Receive will spawn to pull
	// messages concurrently. If NumGoroutines is less than 1, it will be treated
	// as if it were DefaultReceiveSettings.NumGoroutines.
	NumGoroutines int

However, while it's true that pubsub spawns only 1 Goroutine to receive messages, it also calls the receiving function in another goroutine. This means all the messages pubsub receives will be processed in parallel.

The task my code works on is both cpu and memory intensive. By pubsub spawning goroutines for every message, the system quickly runs out of memory.

		group.Go(func() error {
			// TODO(jba): call release when the message is available for GC.
			// This considers the message to be released when
			// f is finished, but f may ack early or not at all.
			defer fc.release(len(msg.Data))
			f(ctx2, msg)
			return nil
		})

What's the right way to control the flow of messages? Is there anyway to control the flow so only 1 message is processed at a time? (btw, I realize I can do my own flow control using a channel, I was curious the point of NumGoroutine if every message spawns a new goroutine.)

@zhenjl
Copy link
Author

zhenjl commented Jul 30, 2017

I just found that I can set MaxOutstandingMessages to do flow control so will test that. Maybe this is more of a documentation issue to make it more clear about the behavior?

@pongad pongad self-assigned this Jul 31, 2017
@pongad pongad added documentation api: pubsub Issues related to the Pub/Sub API. labels Jul 31, 2017
@pongad
Copy link
Contributor

pongad commented Jul 31, 2017

cc @jba

I think this is a mix of doc and naming. The name NumGoroutines makes it sound like you're controlling the total number of goroutines while you're actually controlling the number of pullers. Jonathan, do you think NumPullers work? I'm not sure if we should break folks over this.

@jba
Copy link
Contributor

jba commented Aug 1, 2017

Part of the doc for Receive says

Receive calls f concurrently from multiple goroutines. It is encouraged to process messages synchronously in f, even if that processing is relatively time-consuming; Receive will spawn new goroutines for incoming messages, limited by MaxOutstandingMessages and MaxOutstandingBytes in ReceiveSettings.

So I think it's clear if you look around enough. But we should mention that in the NumGoroutines doc. I'll take care of it next week.

@jba jba assigned jba and unassigned pongad Aug 1, 2017
@jba jba added this to the Fixit 2017 Q3 milestone Aug 1, 2017
@jba
Copy link
Contributor

jba commented Aug 9, 2017

@zhenjl Please take a look at https://code-review.googlesource.com/15690 and let us know if that would have helped you.

@zhenjl
Copy link
Author

zhenjl commented Aug 9, 2017

LGTM!

That is much more clear and would have helped me. Thank you!

yoshi-automation added a commit that referenced this issue Apr 13, 2022
This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#793

Changes:

fix(compute): remove proto3_optional from parent_id (#712)

  Source-Link: googleapis/googleapis@fd16b6a

docs(firestore): clarifications for filters
  PiperOrigin-RevId: 441242400
  Source-Link: googleapis/googleapis@9ef0015

fix(compute): replace missing REQUIRED for parent_id (#711)

  Source-Link: googleapis/googleapis@4bb6fd6

docs(bigquery/reservation): cleanup and clarifications feat: add UpdateAssignment method feat: add "concurrency" and "multi_region_auxiliary" in reservation feat: add preferred table.
  PiperOrigin-RevId: 440912466
  Source-Link: googleapis/googleapis@7ab53ca

fix(speech): use full link in comment to fix JSDoc broken link
  Committer: @summer-ji-eng
  PiperOrigin-RevId: 440481666
  Source-Link: googleapis/googleapis@6a21110

chore(servicedirectory): remove unused imports
  PiperOrigin-RevId: 440391736
  Source-Link: googleapis/googleapis@5d84222

docs(datacatalog): update taxonomy display_name comment feat: added Dataplex specific fields
  PiperOrigin-RevId: 440386238
  Source-Link: googleapis/googleapis@99fd8be

chore(pubsublite): remove unused imports
  PiperOrigin-RevId: 440384445
  Source-Link: googleapis/googleapis@97350b3

feat(securitycenter): Add next_steps field to finding's list of attributes
  PiperOrigin-RevId: 440383959
  Source-Link: googleapis/googleapis@6a276f6

fix(dataproc)!: Move `yarn_config` into a `oneof`
  Committer: @Harwayne
  PiperOrigin-RevId: 440226213
  Source-Link: googleapis/googleapis@c782e45

fix(dataproc)!: Remove `temp_bucket` from VirtualClusterConfig, as its value was not used
  Committer: @Harwayne
  PiperOrigin-RevId: 440224385
  Source-Link: googleapis/googleapis@afc5066

feat(monitoring/dashboard): Sync public protos with latests public api state. This adds support for collapsible groups, filters, labels, drilldowns, logs panels and tables
  PiperOrigin-RevId: 440139643
  Source-Link: googleapis/googleapis@2bff0f3
yoshi-automation added a commit that referenced this issue Apr 13, 2022
This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#793

Changes:

fix(compute): remove proto3_optional from parent_id (#712)

  Source-Link: googleapis/googleapis@fd16b6a

docs(firestore): clarifications for filters
  PiperOrigin-RevId: 441242400
  Source-Link: googleapis/googleapis@9ef0015

fix(compute): replace missing REQUIRED for parent_id (#711)

  Source-Link: googleapis/googleapis@4bb6fd6

docs(bigquery/reservation): cleanup and clarifications feat: add UpdateAssignment method feat: add "concurrency" and "multi_region_auxiliary" in reservation feat: add preferred table.
  PiperOrigin-RevId: 440912466
  Source-Link: googleapis/googleapis@7ab53ca

fix(speech): use full link in comment to fix JSDoc broken link
  Committer: @summer-ji-eng
  PiperOrigin-RevId: 440481666
  Source-Link: googleapis/googleapis@6a21110

chore(servicedirectory): remove unused imports
  PiperOrigin-RevId: 440391736
  Source-Link: googleapis/googleapis@5d84222

docs(datacatalog): update taxonomy display_name comment feat: added Dataplex specific fields
  PiperOrigin-RevId: 440386238
  Source-Link: googleapis/googleapis@99fd8be

chore(pubsublite): remove unused imports
  PiperOrigin-RevId: 440384445
  Source-Link: googleapis/googleapis@97350b3

feat(securitycenter): Add next_steps field to finding's list of attributes
  PiperOrigin-RevId: 440383959
  Source-Link: googleapis/googleapis@6a276f6

fix(dataproc)!: Move `yarn_config` into a `oneof`
  Committer: @Harwayne
  PiperOrigin-RevId: 440226213
  Source-Link: googleapis/googleapis@c782e45

fix(dataproc)!: Remove `temp_bucket` from VirtualClusterConfig, as its value was not used
  Committer: @Harwayne
  PiperOrigin-RevId: 440224385
  Source-Link: googleapis/googleapis@afc5066

feat(monitoring/dashboard): Sync public protos with latests public api state. This adds support for collapsible groups, filters, labels, drilldowns, logs panels and tables
  PiperOrigin-RevId: 440139643
  Source-Link: googleapis/googleapis@2bff0f3
codyoss pushed a commit that referenced this issue Apr 14, 2022
This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#793

Changes:

fix(compute): remove proto3_optional from parent_id (#712)

  Source-Link: googleapis/googleapis@fd16b6a

docs(firestore): clarifications for filters
  PiperOrigin-RevId: 441242400
  Source-Link: googleapis/googleapis@9ef0015

fix(compute): replace missing REQUIRED for parent_id (#711)

  Source-Link: googleapis/googleapis@4bb6fd6

docs(bigquery/reservation): cleanup and clarifications feat: add UpdateAssignment method feat: add "concurrency" and "multi_region_auxiliary" in reservation feat: add preferred table.
  PiperOrigin-RevId: 440912466
  Source-Link: googleapis/googleapis@7ab53ca

fix(speech): use full link in comment to fix JSDoc broken link
  Committer: @summer-ji-eng
  PiperOrigin-RevId: 440481666
  Source-Link: googleapis/googleapis@6a21110

chore(servicedirectory): remove unused imports
  PiperOrigin-RevId: 440391736
  Source-Link: googleapis/googleapis@5d84222

docs(datacatalog): update taxonomy display_name comment feat: added Dataplex specific fields
  PiperOrigin-RevId: 440386238
  Source-Link: googleapis/googleapis@99fd8be

chore(pubsublite): remove unused imports
  PiperOrigin-RevId: 440384445
  Source-Link: googleapis/googleapis@97350b3

feat(securitycenter): Add next_steps field to finding's list of attributes
  PiperOrigin-RevId: 440383959
  Source-Link: googleapis/googleapis@6a276f6

feat(monitoring/dashboard): Sync public protos with latests public api state. This adds support for collapsible groups, filters, labels, drilldowns, logs panels and tables
  PiperOrigin-RevId: 440139643
  Source-Link: googleapis/googleapis@2bff0f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API.
Projects
None yet
Development

No branches or pull requests

3 participants