-
Notifications
You must be signed in to change notification settings - Fork 513
Add metadata property to configure Batching in Pulsar #1707
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
Conversation
…es in Pulsar Signed-off-by: saberwang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Signed-off-by: saberwang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit tests failing.
Signed-off-by: saberwang <[email protected]>
Thank you for the contribution, @saber-wang 🥳! |
Signed-off-by: saberwang <[email protected]>
Signed-off-by: saberwang <[email protected]>
Signed-off-by: saberwang <[email protected]>
Signed-off-by: saberwang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, waiting for settings to be hardcoded.
Signed-off-by: saberwang <[email protected]>
Signed-off-by: saberwang <[email protected]>
@@ -317,3 +352,14 @@ func (p *Pulsar) formatTopic(topic string) string { | |||
} | |||
return fmt.Sprintf(topicFormat, persist, p.metadata.Tenant, p.metadata.Namespace, topic) | |||
} | |||
|
|||
func formatDuration(durationString string) (time.Duration, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1172 Is there a better way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@saber-wang we can make it simpler by always expecting a number.
The metadata can be batchingMaxPublishDelayMs
to clearly indicate that it should be in milliseconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@berndverst this comment was still pending.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1172 Need to change to such a design?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would have been cleaner and simpler but Bernd has already approved/merged this PR. If we have enough consensus, we can create another PR on top to change that I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shubham1172 I don't have a better idea. I can create a new rp to modify it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @saber-wang. Let's see what Bernd/@daixiang0 have to say.
Signed-off-by: saberwang <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1707 +/- ##
==========================================
+ Coverage 36.37% 36.39% +0.02%
==========================================
Files 166 166
Lines 15488 15517 +29
==========================================
+ Hits 5633 5647 +14
- Misses 9228 9239 +11
- Partials 627 631 +4
Continue to review full report at Codecov.
|
* Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <[email protected]> * sort field Signed-off-by: saberwang <[email protected]> * [pubsub]fix unit test bug Signed-off-by: saberwang <[email protected]> * remove unrelated changes Signed-off-by: saberwang <[email protected]> * Delete hard coded Metadata Signed-off-by: saberwang <[email protected]> * remove .history Signed-off-by: saberwang <[email protected]> * restore .gitignore Signed-off-by: saberwang <[email protected]> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <[email protected]> * fix code format Signed-off-by: saberwang <[email protected]> * formatting code Signed-off-by: saberwang <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Bernd Verst <[email protected]>
* Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <[email protected]> * sort field Signed-off-by: saberwang <[email protected]> * [pubsub]fix unit test bug Signed-off-by: saberwang <[email protected]> * remove unrelated changes Signed-off-by: saberwang <[email protected]> * Delete hard coded Metadata Signed-off-by: saberwang <[email protected]> * remove .history Signed-off-by: saberwang <[email protected]> * restore .gitignore Signed-off-by: saberwang <[email protected]> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <[email protected]> * fix code format Signed-off-by: saberwang <[email protected]> * formatting code Signed-off-by: saberwang <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]>
* Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <[email protected]> * sort field Signed-off-by: saberwang <[email protected]> * [pubsub]fix unit test bug Signed-off-by: saberwang <[email protected]> * remove unrelated changes Signed-off-by: saberwang <[email protected]> * Delete hard coded Metadata Signed-off-by: saberwang <[email protected]> * remove .history Signed-off-by: saberwang <[email protected]> * restore .gitignore Signed-off-by: saberwang <[email protected]> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <[email protected]> * fix code format Signed-off-by: saberwang <[email protected]> * formatting code Signed-off-by: saberwang <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Chen Cong <[email protected]>
* Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <[email protected]> * sort field Signed-off-by: saberwang <[email protected]> * [pubsub]fix unit test bug Signed-off-by: saberwang <[email protected]> * remove unrelated changes Signed-off-by: saberwang <[email protected]> * Delete hard coded Metadata Signed-off-by: saberwang <[email protected]> * remove .history Signed-off-by: saberwang <[email protected]> * restore .gitignore Signed-off-by: saberwang <[email protected]> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <[email protected]> * fix code format Signed-off-by: saberwang <[email protected]> * formatting code Signed-off-by: saberwang <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Eddie <[email protected]>
* Update readme of bindings (#1690) Signed-off-by: pigletfly <[email protected]> Co-authored-by: Looong Dai <[email protected]> Signed-off-by: Eddie <[email protected]> * Fixing includedHeaders problem with spaces (#1610) Signed-off-by: Ben Kotvis <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Co-authored-by: Looong Dai <[email protected]> Signed-off-by: Eddie <[email protected]> * Simplify vault token read (#1560) * Simplify vault token get Signed-off-by: zhangchao <[email protected]> * fix lint Signed-off-by: zhangchao <[email protected]> * update tests Signed-off-by: zhangchao <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Signed-off-by: Eddie <[email protected]> * GH-1609 : Fix for MongoDB Atlas conn strings Added recommended fix in the issue Signed-off-by: Eddie <[email protected]> * updating the comment based on PR feedback Signed-off-by: Eddie <[email protected]> * Initial Certification test for eventhubs binding [incomplete] (#1670) * certification test for eventhubs binding Signed-off-by: tanvigour <[email protected]> * modified go.mod and go.sum Signed-off-by: tanvigour <[email protected]> * Add connection string testing Signed-off-by: tanvigour <[email protected]> * iothub testing Signed-off-by: tanvigour <[email protected]> * address feedback and run test Signed-off-by: tanvigour <[email protected]> * Install Azure CLI IOT hub extension Signed-off-by: Bernd Verst <[email protected]> * make modtidy-all Signed-off-by: Bernd Verst <[email protected]> * covering all eventhubs test cases Signed-off-by: tanvigour <[email protected]> * dependency changes after go modtidy-all Signed-off-by: tanvigour <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Co-authored-by: Looong Dai <[email protected]> Signed-off-by: Eddie <[email protected]> * Use revive instead of golint (#1685) Signed-off-by: pigletfly <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Signed-off-by: Eddie <[email protected]> * Updated to Go 1.18 (#1697) * Updated to Go 1.18 Signed-off-by: Alessandro (Ale) Segala <[email protected]> * Added go.work file With Go 1.18, this allows gopls (the Go language server used for example in VS Code) to work inside test apps too. See: https://go.dev/doc/tutorial/workspaces Signed-off-by: ItalyPaleAle <[email protected]> Signed-off-by: ItalyPaleAle <[email protected]> * Removed go.work Signed-off-by: ItalyPaleAle <[email protected]> * 💄 Signed-off-by: ItalyPaleAle <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Eddie <[email protected]> * Add metadata property to configure Batching in Pulsar (#1707) * Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <[email protected]> * sort field Signed-off-by: saberwang <[email protected]> * [pubsub]fix unit test bug Signed-off-by: saberwang <[email protected]> * remove unrelated changes Signed-off-by: saberwang <[email protected]> * Delete hard coded Metadata Signed-off-by: saberwang <[email protected]> * remove .history Signed-off-by: saberwang <[email protected]> * restore .gitignore Signed-off-by: saberwang <[email protected]> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <[email protected]> * fix code format Signed-off-by: saberwang <[email protected]> * formatting code Signed-off-by: saberwang <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Eddie <[email protected]> * Fix 4529: Ignore Subscribe/Get wrong redis configuration type keys. (#1693) * fix: 4529 Signed-off-by: LaurenceLiZhixin <[email protected]> * Fix: add test does not throw error for wrong type during get all test case of redis configuration Signed-off-by: LaurenceLiZhixin <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Co-authored-by: Ian Luo <[email protected]> Signed-off-by: Eddie <[email protected]> * Signed-off-by: Eddie Wassef <[email protected]> Fixing leading newline in linter error Signed-off-by: Eddie <[email protected]> * Implment a secret store based on Huawei CSMS (#1710) Signed-off-by: Chen Cong <[email protected]> Co-authored-by: Chen Cong <[email protected]> Signed-off-by: Eddie <[email protected]> * Add yet another missing secret to eventhub binding cert test (#1713) Signed-off-by: Bernd Verst <[email protected]> Signed-off-by: Eddie <[email protected]> * Support custom queueEndpoint in Azure Storage Queues (#1692) * Support custom queueEndpoint in Azure Storage Queues Signed-off-by: Janusz Dziurzynski <[email protected]> * run gofmt Signed-off-by: Janusz Dziurzynski <[email protected]> * Add "Url" to JSON field name for clarity Suggested by @msfussell in dapr/docs#2424 Signed-off-by: Janusz Dziurzynski <[email protected]> Signed-off-by: Eddie <[email protected]> * Refactory kafka binding to reuse the kafka common code extracting from kafka pubsub component (#1696) * refactory kafka pubsub code to extract common kafka code for reuse Signed-off-by: Sky Ao <[email protected]> * fix lint;add unit test for subscribeAdapter Signed-off-by: Sky Ao <[email protected]> * move topics filed from internal kafak struct to pubsub kafka struct, since in input binding the topics will confiured in metadata Signed-off-by: Sky Ao <[email protected]> * reuse internal kafka code for bindings Signed-off-by: Sky Ao <[email protected]> * add redis standalone_test back which is delete by mistaken Signed-off-by: Sky Ao <[email protected]> * small code improvement to trigger test Signed-off-by: Sky Ao <[email protected]> * add license headers Signed-off-by: Sky Ao <[email protected]> * try to set disbaleTls to true to verify the kafka connection fail Signed-off-by: Sky Ao <[email protected]> * don't enable consum retry in kafka binding component;if authenticaion is disabled, need not set TLSDisable at the same time; Signed-off-by: Sky Ao <[email protected]> * fix lint Signed-off-by: Sky Ao <[email protected]> Co-authored-by: Loong Dai <[email protected]> Signed-off-by: Eddie <[email protected]> * Add topic metadata for mqtt input binding and support user defined topic for mqtt output binding (#1674) * feat(bindings/mqtt): add data incoming topic to metadata Signed-off-by: lotuc <[email protected]> * feat(bindings/mqtt): support user defined topic on create action Signed-off-by: lotuc <[email protected]> * chore(bindings/mqtt): add integration test and topic response check test Signed-off-by: lotuc <[email protected]> * fix(bindings/mqtt): ignore misspell linting error for word mosquitto Signed-off-by: lotuc <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Signed-off-by: Eddie <[email protected]> * Expire -> ExpiryInSeconds (#1721) Signed-off-by: seeflood <[email protected]> Signed-off-by: Eddie <[email protected]> * running gofmt -s -w state/mongodb/mongodb.go Signed-off-by: Eddie <[email protected]> * Update mongodb.go Co-authored-by: Wang Bing <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Ben Kotvis <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Co-authored-by: Taction <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Co-authored-by: tanvigour <[email protected]> Co-authored-by: Alessandro (Ale) Segala <[email protected]> Co-authored-by: saber-wang <[email protected]> Co-authored-by: Laurence <[email protected]> Co-authored-by: Ian Luo <[email protected]> Co-authored-by: Chock Chen <[email protected]> Co-authored-by: Chen Cong <[email protected]> Co-authored-by: Janusz Dziurzynski <[email protected]> Co-authored-by: Sky Ao <[email protected]> Co-authored-by: lotuc <[email protected]> Co-authored-by: seeflood <[email protected]>
* Updated to Go 1.18 Signed-off-by: Alessandro (Ale) Segala <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * Updated Azure SDKs that are on track2 Includes some minor refactoring of auth code Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * Updated Service Bus components to track2 SDK Co-authored-by: halspang <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * Initial Certification test for eventhubs binding [incomplete] (#1670) * certification test for eventhubs binding Signed-off-by: tanvigour <[email protected]> * modified go.mod and go.sum Signed-off-by: tanvigour <[email protected]> * Add connection string testing Signed-off-by: tanvigour <[email protected]> * iothub testing Signed-off-by: tanvigour <[email protected]> * address feedback and run test Signed-off-by: tanvigour <[email protected]> * Install Azure CLI IOT hub extension Signed-off-by: Bernd Verst <[email protected]> * make modtidy-all Signed-off-by: Bernd Verst <[email protected]> * covering all eventhubs test cases Signed-off-by: tanvigour <[email protected]> * dependency changes after go modtidy-all Signed-off-by: tanvigour <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Co-authored-by: Looong Dai <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * Use revive instead of golint (#1685) Signed-off-by: pigletfly <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * Updated to Go 1.18 (#1697) * Updated to Go 1.18 Signed-off-by: Alessandro (Ale) Segala <[email protected]> * Added go.work file With Go 1.18, this allows gopls (the Go language server used for example in VS Code) to work inside test apps too. See: https://go.dev/doc/tutorial/workspaces Signed-off-by: ItalyPaleAle <[email protected]> Signed-off-by: ItalyPaleAle <[email protected]> * Removed go.work Signed-off-by: ItalyPaleAle <[email protected]> * 💄 Signed-off-by: ItalyPaleAle <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * 💄 & 🧹 Signed-off-by: ItalyPaleAle <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * Add metadata property to configure Batching in Pulsar (#1707) * Add metadata property to configure BatchingMaxSize&batchingMaxMessages in Pulsar Signed-off-by: saberwang <[email protected]> * sort field Signed-off-by: saberwang <[email protected]> * [pubsub]fix unit test bug Signed-off-by: saberwang <[email protected]> * remove unrelated changes Signed-off-by: saberwang <[email protected]> * Delete hard coded Metadata Signed-off-by: saberwang <[email protected]> * remove .history Signed-off-by: saberwang <[email protected]> * restore .gitignore Signed-off-by: saberwang <[email protected]> * Hard coding default values and adding 'BatchingMaxPublishDelay' metadata Signed-off-by: saberwang <[email protected]> * fix code format Signed-off-by: saberwang <[email protected]> * formatting code Signed-off-by: saberwang <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * This test can't work with track2 SDKs The methods to create a message with a body are not exported Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * 🧹 Signed-off-by: Alessandro Segala (ItalyPaleAle) <[email protected]> * There's such thing as too much logging Signed-off-by: ItalyPaleAle <[email protected]> * Refactored subscription.go This greatly simplifies certain parts of the code, reducing the number of goroutines and likely improving performance. Performance for end-users improves too as there's no need anymore to pause for 2 seconds every time that we reach `maxActiveMessages`. Additionally, with this change the config options `prefetchCount` and `maxActiveMessagesRecoveryInSec` are removed as unnecessary anymore. Signed-off-by: ItalyPaleAle <[email protected]> * 💄 Signed-off-by: ItalyPaleAle <[email protected]> * Fixed pubsub tests Signed-off-by: ItalyPaleAle <[email protected]> * These packages should have never been upgraded Signed-off-by: ItalyPaleAle <[email protected]> * Ensuring we don't fetch 1 message more than max active Signed-off-by: ItalyPaleAle <[email protected]> * Adding configurable timeout for servicebusqueues operations Signed-off-by: ItalyPaleAle <[email protected]> * Persistent connection for invoking SB queues Signed-off-by: ItalyPaleAle <[email protected]> * Reverted Event Hub SDK update Signed-off-by: ItalyPaleAle <[email protected]> * Revert "Reverted Event Hub SDK update" This reverts commit 212220a. Signed-off-by: ItalyPaleAle <[email protected]> * Fix Azure deploy for users with a first.last email Signed-off-by: ItalyPaleAle <[email protected]> * Added some sleep that should help reduce flakiness in eventhubs binding cert test Signed-off-by: ItalyPaleAle <[email protected]> * Changed servicebusqueue cert test In case of a failure in the handler (ie. users' code), the message should be correctly re-enqueued, which means that messages will be re-delivered later and won't be in order. Signed-off-by: ItalyPaleAle <[email protected]> Co-authored-by: halspang <[email protected]> Co-authored-by: tanvigour <[email protected]> Co-authored-by: Bernd Verst <[email protected]> Co-authored-by: Yaron Schneider <[email protected]> Co-authored-by: Looong Dai <[email protected]> Co-authored-by: Wang Bing <[email protected]> Co-authored-by: saber-wang <[email protected]>
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #1671
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: