There is no code to use parameter 'event_publisher_pub_hwm' in saltstack-2016.3 #38674

Closed
jackywu opened this Issue Jan 11, 2017 · 6 comments

Projects

None yet

4 participants

@jackywu
Contributor
jackywu commented Jan 11, 2017 edited

Description of Issue/Question

I'am researching the meaning of parameter event_publisher_pub_hwm and salt_event_pub_hwm,and I find them were set in DEFAULT_MASTER_OPTS and DEFAULT_MINION_OPTS in salt/config/init.py`, but no code to use them. Did these parameters not work?

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

Steps to Reproduce Issue

(Include debug logs if possible and relevant.)

Versions Report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

saltstack-2016.3 , even saltstack-2016.11

@Ch3LL
Contributor
Ch3LL commented Jan 11, 2017 edited

@jackywu looks like it was removed with this PR: #29294

This is a little over my head so i'm going to bring in a couple more people who might be able to explain whether this is on purpose to remove these options and we need to update our docs or if we need to re-add this option.

ping @skizunov @DmitryKuzmenko or @cachedout do you know whether we need to add this option bck into the code or document that it is not longer availabe?

@Ch3LL Ch3LL added this to the Blocked milestone Jan 11, 2017
@skizunov
Contributor

These options were used back in the day when the event system was implemented using ZMQ. The event system was moved to use TCP / tornado in PR #29294. After that PR was merged, these options were obsoleted. I would say it is safe to remove them. I apologize for not removing them in that PR.

@jackywu
Contributor
jackywu commented Jan 12, 2017

@skizunov , Will TCP transport be the recommended and default transport implementation in the future?Will the SaltStack community give up ZeroMQ ?

@skizunov
Contributor

@jackywu : When I say TCP / tornado, I really mean localhost sockets and tornado. It is similar to how the TCP transport works, but the connections are local and it is independent of the external TCP transport. Hence the event system will work regardless of which external transport (eg ZeroMQ or TCP) is in use.

As for the future of the external transports for Salt, (eg TCP vs ZeroMQ) I cannot speak to that since I do not work for SaltStack, I am an external contributor.

@Ch3LL
Contributor
Ch3LL commented Jan 12, 2017

I can speak to the future of zeromq. As you can see in this issue #35724 the intention is to change to the tcp tranpsport as default in the future but NOT to get rid of zmq.

Also thanks @skizunov for chiming in here. Seems we just need to remove this from the docs. Thanks

@Ch3LL Ch3LL modified the milestone: Approved, Blocked Jan 12, 2017
@rallytime
Contributor

@jackywu and @skizunov I have removed these two options from the config/__init__.py file in #38723. The only other references I could find to these options were in the man pages. The man pages are autogenerated every so often, so when those are regenerated again, those references will be removed as well.

@rallytime rallytime self-assigned this Jan 13, 2017
@rallytime rallytime modified the milestone: Nitrogen 2, Approved Jan 13, 2017
@rallytime rallytime added a commit that closed this issue Jan 17, 2017
@rallytime rallytime Remove "event_publisher_pub_hwm" and "salt_event_pub_hwm" from config…
…/__init__.py

These options were used back when the event system was based on ZMQ. As of PR
PR #29294, these options are no longer needed and are obsolete. PR #29294 changed
the event system transport from ZMQ to TCP/tornado.

There are no other mentions of these configuration options in the salt code base
except in the man pages. Once the man pages are regenerated, those references will
be removed.

Fixes #38674
706c885
@rallytime rallytime closed this in 706c885 Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment