Skip to content

Reuse channels#10298

Merged
thatch45 merged 2 commits into
saltstack:developfrom
jacksontj:channelreuse
Feb 10, 2014
Merged

Reuse channels#10298
thatch45 merged 2 commits into
saltstack:developfrom
jacksontj:channelreuse

Conversation

@jacksontj
Copy link
Copy Markdown
Contributor

One of the things i've noticed with a fairly large salt install is that the majority of the master's time seems to be authenticating requests. After some digging it is apparent that we end up creating and destroying zmq channels all over creation. This is an attempt to allow for session re-use. The objective is to re-use the same channel for all requests in a single execution/thread/process-- as having a single long running connection back to the master is a much larger job.

store a cache of the channels so a single pid can re-use a connection
this needs to be per pid as when you fork you keep the original memory
space and we can't control currency across pids
@ghost
Copy link
Copy Markdown

ghost commented Feb 8, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1294/

@ghost
Copy link
Copy Markdown

ghost commented Feb 8, 2014

Test FAILed.
Refer to this link for build results: http://jenkins.saltstack.com/job/salt-pr-build/1295/

thatch45 added a commit that referenced this pull request Feb 10, 2014
@thatch45 thatch45 merged commit 00f231e into saltstack:develop Feb 10, 2014
@s0undt3ch
Copy link
Copy Markdown
Contributor

If you run the tests suite with this changes, you'll notice that this broke it bad. Maybe it needs a different approach?

@jacksontj
Copy link
Copy Markdown
Contributor Author

When I ran it locally i had no test failures, what are you seeing?
On Feb 10, 2014 3:30 PM, "Pedro Algarvio" notifications@github.com wrote:

If you run the tests suite with this changes, you'll notice that this
broke it bad. Maybe it needs a different approach?

Reply to this email directly or view it on GitHubhttps://github.com//pull/10298#issuecomment-34704571
.

@s0undt3ch
Copy link
Copy Markdown
Contributor

http://jenkins.saltstack.com/job/salt-rs-arch/1623/artifact/salt-runtests.log look at this log, the traceback in it

@jacksontj
Copy link
Copy Markdown
Contributor Author

I'll have to take a look at this.. my first guess would be some problem
with key rotation maybe? If you want we can revert my commit in develop and
i'll work on my feature branch :)

On Mon, Feb 10, 2014 at 3:40 PM, Pedro Algarvio notifications@github.comwrote:

http://jenkins.saltstack.com/job/salt-rs-arch/1623/artifact/salt-runtests.loglook at this log, the traceback in it

Reply to this email directly or view it on GitHubhttps://github.com//pull/10298#issuecomment-34706387
.

@s0undt3ch
Copy link
Copy Markdown
Contributor

If #10325 get's merged, these commits will be reverted.

We do need some stability in our tests suite in order to prepare for 2014.1 ...

But yes! Please get that feature branch running smoothly, we do want Salt to become more and more performant. Sorry for the troubles...

@jacksontj
Copy link
Copy Markdown
Contributor Author

No worries, sorry about the breakage. Jenkins was messed up when i
submitted the pull req
On Feb 10, 2014 4:07 PM, "Pedro Algarvio" notifications@github.com wrote:

If #10325 #10325 get's merged,
these commits will be reverted.

We do need some stability in our tests suite in order to prepare for
2014.1 ...

But yes! Please get that feature branch running smoothly, we do want Salt
to become more and more performant. Sorry for the troubles...

Reply to this email directly or view it on GitHubhttps://github.com//pull/10298#issuecomment-34711759
.

@jacksontj
Copy link
Copy Markdown
Contributor Author

Unable to reproduce locally-- so i've submitted another pull request
(#10332) to see if jenkins can give me anything more :/

On Mon, Feb 10, 2014 at 4:16 PM, Thomas Jackson jacksontj.89@gmail.comwrote:

No worries, sorry about the breakage. Jenkins was messed up when i
submitted the pull req
On Feb 10, 2014 4:07 PM, "Pedro Algarvio" notifications@github.com
wrote:

If #10325 #10325 get's merged,
these commits will be reverted.

We do need some stability in our tests suite in order to prepare for
2014.1 ...

But yes! Please get that feature branch running smoothly, we do want Salt
to become more and more performant. Sorry for the troubles...

Reply to this email directly or view it on GitHubhttps://github.com//pull/10298#issuecomment-34711759
.

@s0undt3ch
Copy link
Copy Markdown
Contributor

@basepi let's not cherry-pick this just yet

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.

3 participants