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

RemoteDriverProtocol needs multiple instances #505

Closed
jnioche opened this Issue Oct 12, 2017 · 1 comment

Comments

Projects
None yet
1 participant
@jnioche
Member

jnioche commented Oct 12, 2017

This is not an issue when using the SimpleFetcherBolt as each instance has its own ProtocolFactory, however, with the FetcherBolt which has a shared ProtocolFactory for all the fetching threads, we have only one instance of RemoteDriverProtocol running at a time.

This is not a problem for the other protocol implementations as their underlying libs can handle multithreaded access but that won't work with RemoteWebDriver. One option would be to give fetching threads their own instance of ProtocolFactory but that would not be a good idea as this would mean one map and config instances per thread + have as many open connections as we have threads.

A better approach would be to either:

  • (minimal approach) add a parameter so that we create N instances for each selenium.address
  • replace the LinkedBlockingQueue from the SeleniumProtocol abstract class and replace it with a Threadpool or something similar so that we could close old connections etc...

@jnioche jnioche added bug core labels Oct 12, 2017

@jnioche jnioche added this to the 1.7 milestone Oct 12, 2017

@jnioche jnioche closed this Oct 19, 2017

@jnioche jnioche reopened this Oct 19, 2017

@jnioche jnioche closed this in 929a51b Oct 19, 2017

@jnioche

This comment has been minimized.

Show comment
Hide comment
@jnioche

jnioche Oct 19, 2017

Member

Implemented the minimal approach. Use selenium.instances.num: in conf

Member

jnioche commented Oct 19, 2017

Implemented the minimal approach. Use selenium.instances.num: in conf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment