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.

Sign up for GitHub

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

Jump to bottom

chore(broker): expose more backpressure algorithms and configurations #4610

Merged
merged 1 commit into from
Jun 2, 2020
Merged

chore(broker): expose more backpressure algorithms and configurations #4610

merged 1 commit into from
Jun 2, 2020

Conversation

deepthidevaki
Copy link
Contributor

Description

  • Expose more backpressure algorithms and a subset of their configuration parameters.
  • Add backpressure configs to broker.yaml.template for reference

Related issues

closes #4595

Pull Request Checklist

  • All commit messages match our commit message guidelines
  • The submitting code follows our code style
  • If submitting code, please run mvn clean install -DskipTests locally before committing

@deepthidevaki deepthidevaki requested a review from pihme May 27, 2020 08:57
@deepthidevaki deepthidevaki force-pushed the 4595-port-backpressure-config branch from cb3d6b3 to 9e318cb Compare May 27, 2020 09:01
pihme
pihme suggested changes May 27, 2020
View reviewed changes
Copy link
Contributor

@pihme pihme left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

Mostly optional suggestions. Feel free to ignore or implement them, and mark them as resolved

Comments where I didn't put optional in front: I would like to review changes for those.

broker/src/main/java/io/zeebe/broker/Broker.java Outdated Show resolved Hide resolved
broker/src/main/java/io/zeebe/broker/system/configuration/AIMDCfg.java Outdated Show resolved Hide resolved
broker/src/main/java/io/zeebe/broker/system/configuration/AIMDCfg.java Outdated Show resolved Hide resolved
broker/src/main/java/io/zeebe/broker/system/configuration/AIMDCfg.java Outdated Show resolved Hide resolved
broker/src/main/java/io/zeebe/broker/transport/backpressure/PartitionAwareRequestLimiter.java Show resolved Hide resolved
dist/src/main/config/broker.yaml.template Outdated Show resolved Hide resolved
dist/src/main/config/broker.yaml.template Show resolved Hide resolved
dist/src/main/config/broker.yaml.template Show resolved Hide resolved
dist/src/main/config/broker.yaml.template Show resolved Hide resolved
broker/src/main/java/io/zeebe/broker/system/configuration/BackpressureCfg.java Outdated Show resolved Hide resolved
@pihme
Copy link
Contributor

pihme commented May 27, 2020

in case you didn't have many dealings with SonarCloud before: see Gateway line 46 for suppression of SonarCloud warnings. The code is in the upper right corner after you click on "Why is this an issue?"

@deepthidevaki deepthidevaki requested a review from pihme May 29, 2020 11:11
@deepthidevaki
Copy link
Contributor Author

@pihme I have addressed your comments. It seems it is not possible to suppress duplicate code warnings in SonarCloud. If you know how to fix it let me know.

pihme
pihme approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@pihme pihme left a comment

Choose a reason for hiding this comment

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

Two additional comments.

broker/src/main/java/io/zeebe/broker/system/configuration/backpressure/Gradient2Cfg.java Outdated Show resolved Hide resolved
broker/src/main/java/io/zeebe/broker/system/configuration/backpressure/Gradient2Cfg.java Outdated
public class Gradient2Cfg {

private int minLimit = 10;
private int initialLimit = 20;
private double rttTolerance = 2.0;
private int longWindow = 600;

@SuppressWarnings("common-java:DuplicatedBlocks")
Copy link
Contributor

Choose a reason for hiding this comment

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

that does not work. Suppress warnings is a general annotation. To tell it to ignore sonar qube warnings, you would need
@SuppressWarnings("squid:common-java:DuplicatedBlocks") where squid stands for SonarQube Unique ID, I guess.

Not sure whether you need to have common-java in or not. When I used it, I didn't need it. I guess SonarQube knows it is looking at Java already.

Alternatively, I also found:

For instance, you can add the following property to your scanner configuration:
sonar.cpd.exclusions=path/to/your/package/*.java
on this page: https://stackoverflow.com/questions/52865737/how-do-i-ignore-duplicated-code-report-in-sonar

I guess this could be set somewehere in the pom files, but I haven't done it yet.

@pihme
Copy link
Contributor

pihme commented May 29, 2020

Nice work! Looking forward to trying different algorithms soon :-)

@deepthidevaki deepthidevaki force-pushed the 4595-port-backpressure-config branch from eec822a to ba7be38 Compare June 2, 2020 06:28
@deepthidevaki deepthidevaki mentioned this pull request Jun 2, 2020
Closed
@deepthidevaki
Copy link
Contributor Author

bors r+

@zeebe-bors
Copy link
Contributor

zeebe-bors bot commented Jun 2, 2020

Build succeeded

@zeebe-bors zeebe-bors bot merged commit 8538257 into develop Jun 2, 2020
@zeebe-bors zeebe-bors bot deleted the 4595-port-backpressure-config branch June 2, 2020 06:53
github-merge-queue bot pushed a commit that referenced this pull request Mar 14, 2024
@mihail-ca
feat(backend): Endpoint to search decision instances (#4893)
6be056f 
* feat(backend): Endpoint to search decision instances

feat(backend): Endpoint to search decision instances

- added model and controller
- added tests

Closes #4610
github-merge-queue bot pushed a commit that referenced this pull request Apr 16, 2024
@renovate
chore(deps): update hashicorp/vault-action digest to a727ce2 (master) (…
3f9891b 
…#4610)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pihme pihme pihme approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose more backpressure algorithms and config in the broker.
4 participants
@deepthidevaki @pihme @npepinpe @lenaschoenburg