feat: move on-demand subscriber to batch service #952
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ubisoft-potato
force-pushed
the
upgrade-pubsub-arch
branch
from
de676bc
to
d0f0933
Compare
Ubisoft-potato
changed the title
cre8ivejp
reviewed
May 28, 2024
| @@ -1,16 +1,16 @@ | |||
| // Copyright 2023 The Bucketeer Authors. | |||
| // Copyright 2024 The Bucketeer Authors. | |||
There was a problem hiding this comment.
nit: It shouldn't introduce an extra space here.
| bigQueryBatchSize: | ||
| timezone: | ||
| goalCountEventOPSPersister: | ||
| flushSize: 100 |
There was a problem hiding this comment.
Please use the following default settings for all the new persisters.
flushSize: 20
flushInterval: 5
flushTimeout: 30| project: | ||
| topic: | ||
| subscription: | ||
| pullerNumGoroutines: 5 |
There was a problem hiding this comment.
Please use the following default settings for all the new persisters.
pullerNumGoroutines: 5
pullerMaxOutstandingMessages: 1000
pullerMaxOutstandingBytes: 100000000
maxMps: 100
workerNum: 1
checkInterval: 10
cre8ivejp
reviewed
May 29, 2024
pkg/batch/subscriber/subscriber.go
Outdated
| @@ -64,7 +74,7 @@ type Configuration struct { | |||
| WorkerNum int `json:"workerNum"` | |||
| } | |||
|
|
|||
| type Subscriber struct { | |||
| type NormalSubscriber struct { | |||
There was a problem hiding this comment.
Let's keep the the name Subscriber.
Also, please make it private.
| @@ -103,7 +103,7 @@ func getCodeFromError(err error) string { | |||
| return codeUnknown | |||
| } | |||
|
|
|||
| func registerMetrics(r metrics.Registerer) { | |||
| func RegisterMetrics(r metrics.Registerer) { | |||
There was a problem hiding this comment.
Do we need to make this public?
There was a problem hiding this comment.
Yes, since the events dwh persisters need to use bigquery writer, but we can't register the metrics twice, so we have to register bigquery writer metrics when the batch service starting
| @@ -1,21 +1,22 @@ | |||
| // Copyright 2024 The Bucketeer Authors. | |||
| // Copyright 2024 The Bucketeer Authors. | |||
| @@ -1,33 +1,33 @@ | |||
| // Copyright 2024 The Bucketeer Authors. | |||
| // Copyright 2024 The Bucketeer Authors. | |||
Ubisoft-potato
force-pushed
the
upgrade-pubsub-arch
branch
from
1e1d74b
to
76d0326
Compare
This reverts commit 12d5b8c.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #405
This PR will upgrade multi PubSub architecture to support on-demand subscriber:
Key Changes
Refactor event persisters
Move storage pkg
pkg/batch/storagedirectory. [1] [2] [3] [4]