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

Selenium 3.0.0-beta3 node maxSession = 1 causes hub registration of default 5 instead of 1 #2727

Closed
james-crocker opened this issue Sep 6, 2016 · 3 comments

Comments

@james-crocker
Copy link

Meta -

OS: Windows 10
Selenium Version:
3.0.0-beta3 node (win10) and 3.0.0-beta3 hub (linux)
Browser:
all
Browser Version:
any

Expected Behavior -

Setting node maxSession = 1 should restrict node's active sessions to one.

Actual Behavior -

Setting node maxSession = 1 via command line argument or json causes registration at hub to be 5. Consequently the hub will spawn 5 concurrent sessions. Setting to maxSession = 2 or higher registers correctly on the hub.

Steps to reproduce -

Set -maxSession 1 or in json configuration "maxSession": 1 causes registration at hub of maxSession: 5. Unregister node, set maxSession to 2 and registration correctly sets 2 and honors that value.

@james-crocker
Copy link
Author

Related: setting hub maxSession = 1 results in maxSession being 1 (and for values >= 1).
However, the nodes registered maxSession is overridden by the hub maxSession if the hub's maxSession > node's maxSession.
So... (=> is the registered grid hub settings)
hubMaxSession=1, nodeMaxSession=2 (1 triggers default 5) => hubMaxSession:1, nodeMaxSession:2
hubMaxSession=2, nodeMaxSession=2 => hubMaxSession:2, nodeMaxSession:2
hubMaxSession=3, nodeMaxSession=2 => hubMaxSession:3, nodeMaxSession:3
etc..

@mach6
Copy link
Member

mach6 commented Sep 6, 2016

Thanks @james-crocker. I've seen this and similar issues. Will look into it.

@mach6
Copy link
Member

mach6 commented Sep 7, 2016

@lukeis The issue here is two-fold;

  1. maxSession is updated on merge() iff the new value != 1
  2. 3.0.0-beta changes the actual configuration precedence for remote proxies. In 2.x remote configuration overrides hub/registry configuration for collisions. In 3.0.0-beta hub/registry configuration overrides remote configuration for collisions. See mergeConfig in https://github.com/SeleniumHQ/selenium/blob/selenium-2.53.0/java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java#L233-L250 for 2.x and compare that to https://github.com/SeleniumHQ/selenium/blob/master/java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java#L106-L110 in 3.0.0-beta

I have a pending change set but need sync up with you at greater detail on the second part. I believe it is the root of this and other similar issues. Furthermore, I believe we should revert to 2.x behavior

mach6 added a commit to mach6/selenium that referenced this issue Sep 8, 2016
- Revert BaseRemoteProxy configuration load behavior
  to Sel 2.x logic (seed from registry, then overlap
  with the remote request)
- Combine -jettyThreads (hidden, used by standalone
  and node) and -jettyMaxThreads (not hidden, used
  by hub) into -jettyMaxThreads which is NOT hidden
  and is used by standalone, node, and hub
Additional changes;
- Fix for 'id' which should have a default value
  applied (according to its usage doc)
- new GridHubConfiguration() - the role should
  always be "hub"
- new GridNodeConfiguration() - the role should
  always be "node"
- Configuration merge() behavior update/change;
  - don't merge null 'other' values (pre-existing)
  - don't merge empty collections or maps (new)
- Add tests
@lukeis lukeis closed this as completed in 031456c Sep 8, 2016
@lock lock bot locked and limited conversation to collaborators Aug 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants