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

Enable ZMQ between GNMI and Orchanget #16661

Merged
merged 2 commits into from
Oct 9, 2023

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Sep 22, 2023

Enable ZMQ on gnmi and orchagent

Why I did it

Improve GNMI API performance for Dash resources

How I did it

Modify gnmi and orchagent service start script, add ZMQ parameter.

How to verify it

Pass all UT & E2E test
Manually verify with create Dash resources via gnmi API.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Sep 26, 2023

Depends PR merged, close and re-open to trigger rebuild.

@liuh-80 liuh-80 closed this Sep 26, 2023
@liuh-80 liuh-80 reopened this Sep 26, 2023
@liuh-80 liuh-80 marked this pull request as ready for review September 26, 2023 06:17
@liuh-80 liuh-80 requested a review from lguohan as a code owner September 26, 2023 06:17
@@ -69,4 +69,6 @@ else
ORCHAGENT_ARGS+="-m $MAC_ADDRESS"
fi

ORCHAGENT_ARGS+=" -q tcp://127.0.0.1:8100"
Copy link
Collaborator

@qiluo-msft qiluo-msft Sep 28, 2023

Choose a reason for hiding this comment

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

ORCHAGENT_ARGS

On non-dash related platforms, this new arg is not necessary. Could you limit the scope? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I know there is no flag for Dash, I will check and update this PR.

@@ -70,6 +70,8 @@ else
TELEMETRY_ARGS+=" -v=2"
fi

TELEMETRY_ARGS+=" -zmq_address=tcp://127.0.0.1:8100"
Copy link
Collaborator

@qiluo-msft qiluo-msft Sep 28, 2023

Choose a reason for hiding this comment

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

TELEMETRY_ARGS

Could you limit the scope? #Closed

@qiluo-msft qiluo-msft merged commit 6e32600 into sonic-net:master Oct 9, 2023
16 checks passed
@@ -69,4 +69,10 @@ else
ORCHAGENT_ARGS+="-m $MAC_ADDRESS"
fi

# Enable ZMQ for SmartSwitch
LOCALHOST_SUBTYPE=`sonic-db-cli CONFIG_DB hget localhost "subtype"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Here should be

sonic-db-cli CONFIG_DB hget "DEVICE_METADATA|localhost" "subtype"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will update with new PR

@@ -70,6 +70,12 @@ else
TELEMETRY_ARGS+=" -v=2"
fi

# Enable ZMQ for SmartSwitch
LOCALHOST_SUBTYPE=`sonic-db-cli CONFIG_DB hget localhost "subtype"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will update with new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants