LOGBACK-945: extended ServerSocketReceiver and AbstractServerSocketAppender have their respective own thread pools for managing client connections

[SierraGolf]

Previously the ServerSocketReceiver and the AbstractServerSocketAppender and their subclasses were using the context's thread pool for client connections which was effectively limiting the amount of possible client connections to a little less than the value of the CoreConstants.MAX_POOL_SIZE which is 32.

This pull requests extends the above mentioned classes so that they have their own thread pools dedicated for client connections.

The pools can be configured through the following new configuration properties:

• corePoolSize (default: CoreConstants.CORE_POOL_SIZE)
• maxPoolSize (default: CoreConstants.MAX_POOL_SIZE)

Questions:

1. Should I also extend the documentation with this pull request?
2. Should I implement some integration test in addition to the supplied unit tests to verify functional aspects of the new features?

[SierraGolf]

This looks really familiar... :) Is there a way to minimize this code duplication?

I am a big fan of drying up code, but I think in this case it does not really make sense to introduce an abstraction to reduce duplication. It is also difficult to pull out logic to some kind of "helper" (god, I hate this word).

However, if you have an idea, shoot!

[ceki]

Is there a jira associated with this pull request? If not, could you please create one at jira.qos.ch and mention it in this pull request.?

[SierraGolf]

Jup, here you go: http://jira.qos.ch/browse/LOGBACK-945

[tony19]

Questions:

1. Should I also extend the documentation with this pull request?
2. Should I implement some integration test in addition to the supplied unit tests to verify functional aspects of the new features?

"Yes, please" to both questions :)

[SierraGolf]

Hey,

I added some documentation to the site sub-module and a first draft of an integration test. Can you please have a look and tell me if I am going in the right direction?

Currently I am doing a hardcoded sleep (yuck) in order to give all involved parties some time for connection handling and stuff.

Do you guys have an idea how I could use a CountDownLatch or something similar to actually have a feedback from the working components instead of using a sleep?

[SierraGolf]

@ceki / @tony19 could either one of you give me some feedback about my integration testing approach?

[SierraGolf]

Hi @tony19,

I just finished up the integration tests, please have a look. I pretty much did it the way you suggested, but I implemented a new specific instrumented class for each test instead of extending the existing ones. I did not want to bloat them up to much.

I needed to make the RemoteReceiverClient interface public in order to implement the integration test for the AbstractServerSocketAppender. I hope that's ok.

Although the AbstractServerSocketAppender is located in the core module I needed to implement the integration tests in the classic module, because otherwise it wouldn't have been much of an integration test anyway.

I also removed some dead code from the ServerSocketReceiver being a good boy scout and so.

Just one last thing, you might want to configure the host and the port being used for the tests to avoid clashes in CI. They have "clever" defaults but also look for custom overrides in the system properties. You could extend the pom to be able to pipe those parameters through from CLI to the test VM.

[SierraGolf]

@ceki / @tony19 any chance that one of you guys can give me some feedback on this PR?

[SierraGolf]

rebased and applied comments. btw. you might want to regenerate all the headers, we have 2014 already ;)

[SierraGolf]

@tony19 / @ceki any chance this is merged any time soon?

[melezov]

We would love for this patch to be merged. We're hitting the wall of 32 threads and cannot use the vanilla jar in production.