Skip to content

[fix][fn] Support multiple input topics for Go runtime - #20000

Merged
Technoboy- merged 8 commits into
apache:masterfrom
jiangpengcheng:support_multiple_topics_for_go
May 10, 2023
Merged

[fix][fn] Support multiple input topics for Go runtime#20000
Technoboy- merged 8 commits into
apache:masterfrom
jiangpengcheng:support_multiple_topics_for_go

Conversation

@jiangpengcheng

@jiangpengcheng jiangpengcheng commented Apr 3, 2023

Copy link
Copy Markdown
Contributor

Fixes #19981

Master Issue: #xyz

PIP: #xyz

Motivation

The Go runtime doesn't support multi-input topics,

Modifications

Add a map<string, string> field SourceInputSpecs to the GoInstanceConfig, while the key is topic name, and value is the json string of ConsumerSpec, so that the Go instance can unmarshal it to a map[string]ConsumerSpec and support multiple input topics

Verifying this change

  • Make sure that the change passes the CI checks.

  • This change added tests and can be verified as follows:

    • Added integration tests for GO with multiple input topics

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: jiangpengcheng#10

@jiangpengcheng jiangpengcheng changed the title [fix][fn] Support multiple input topics for GO runtime [fix][fn] Support multiple input topics for Go runtime Apr 3, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Apr 3, 2023
@jiangpengcheng

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

1 similar comment
@jiangpengcheng

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@jiangpengcheng
jiangpengcheng force-pushed the support_multiple_topics_for_go branch 2 times, most recently from 1029f31 to a062819 Compare April 7, 2023 02:24
@jiangpengcheng

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@jiangpengcheng
jiangpengcheng force-pushed the support_multiple_topics_for_go branch from 96663c2 to f83ca47 Compare April 10, 2023 09:28
@jiangpengcheng

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@jiangpengcheng
jiangpengcheng force-pushed the support_multiple_topics_for_go branch from f83ca47 to ffa50cc Compare April 11, 2023 01:11
@jiangpengcheng

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

@codecov-commenter

codecov-commenter commented Apr 11, 2023

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.99%. Comparing base (470b674) to head (5706991).
⚠️ Report is 2368 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #20000       +/-   ##
=============================================
+ Coverage     33.13%   72.99%   +39.86%     
- Complexity    12164    31932    +19768     
=============================================
  Files          1497     1868      +371     
  Lines        114032   138294    +24262     
  Branches      12373    15213     +2840     
=============================================
+ Hits          37782   100945    +63163     
+ Misses        71340    29336    -42004     
- Partials       4910     8013     +3103     
Flag Coverage Δ
inttests 24.15% <0.00%> (?)
systests 24.92% <88.88%> (?)
unittests 72.29% <100.00%> (+39.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...pulsar/functions/instance/go/GoInstanceConfig.java 100.00% <100.00%> (+100.00%) ⬆️
.../apache/pulsar/functions/runtime/RuntimeUtils.java 79.07% <100.00%> (+74.37%) ⬆️

... and 1537 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jiangpengcheng

Copy link
Copy Markdown
Contributor Author

/pulsarbot rerun-failure-checks

Comment on lines 692 to 698
if (isTopicPattern || multipleInput) {
if (isTopicPattern) {
inputTopicName = inputTopicName + ".*";
} else {
inputTopicName = inputTopicName + "1," + inputTopicName + "2";
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isTopicPattern || multipleInput) {
if (isTopicPattern) {
inputTopicName = inputTopicName + ".*";
} else {
inputTopicName = inputTopicName + "1," + inputTopicName + "2";
}
}
if (isTopicPattern) {
inputTopicName = inputTopicName + ".*";
} else if (multipleInput) {
inputTopicName = inputTopicName + "1," + inputTopicName + "2";
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@cbornet cbornet modified the milestones: 3.0.0, 3.1.0 Apr 11, 2023
Comment thread pulsar-function-go/conf/conf.go Outdated
Comment on lines 63 to 65
// for backward compatibility
SourceSpecTopic string `json:"sourceSpecsTopic" yaml:"sourceSpecsTopic"`
SourceSchemaType string `json:"sourceSchemaType" yaml:"sourceSchemaType"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should mark Deprecated for fields that's not intended for use

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

@jiangpengcheng
jiangpengcheng requested a review from cbornet April 12, 2023 06:37
@jiangpengcheng
jiangpengcheng force-pushed the support_multiple_topics_for_go branch from 3a9cfb8 to 5706991 Compare April 12, 2023 06:39
@Technoboy-
Technoboy- merged commit f04252b into apache:master May 10, 2023
poorbarcode pushed a commit that referenced this pull request May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Go Pulsar functions recieves message from only one topic even when provided multiple input topics

7 participants