Managed connection pool on Safegard

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

Managed connection pool on Safegard

brunobat@gmail.com
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_


Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

Romain Manni-Bucau
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_


Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

brunobat@gmail.com

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_



Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

Romain Manni-Bucau
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_



Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

John D. Ament-2
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_



Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

brunobat@gmail.com

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_




Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

brunobat@gmail.com

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_





Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

John D. Ament
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_





Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

Romain Manni-Bucau
Hmm

Do we need it? Any config should optionally (as no required dependency) use mp-config.

Now more concretely, any use can impl it very easily - or we can make a simpler constructor if not - so do we need it in the project?

Le lun. 8 oct. 2018 14:41, John D. Ament <[hidden email]> a écrit :
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_





Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

brunobat@gmail.com

The 2 module solution will make integration seamless, Just declare the right artifact and no integration hassle. I think it's always good try to make life simpler to whoever is using the lib.

In relation to mp-config... I see your point. The bean lookup should use it, I guess. I'll do some work on that part.

Cheers.

On 08/10/2018 13:44, Romain Manni-Bucau wrote:
Hmm

Do we need it? Any config should optionally (as no required dependency) use mp-config.

Now more concretely, any use can impl it very easily - or we can make a simpler constructor if not - so do we need it in the project?

Le lun. 8 oct. 2018 14:41, John D. Ament <[hidden email]> a écrit :
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_






Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

brunobat@gmail.com
In reply to this post by John D. Ament

Thanks, will play with the module and add tests.

Cheers

On 08/10/2018 12:41, John D. Ament wrote:
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_






Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

Romain Manni-Bucau
Well the config point is tomee will still fork this bean cause it lust also respect tomee config so id make all services either implicitly looked up or injected in the producer and user has its own config properly instead of creating a module misbehaving OOTB in 90% of the case and needinh another piece of vonfig outside the app.

Gain sounds quite low vs the cost in that perspective.

Le lun. 8 oct. 2018 16:02, Bruno Baptista <[hidden email]> a écrit :

Thanks, will play with the module and add tests.

Cheers

On 08/10/2018 12:41, John D. Ament wrote:
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_






Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

brunobat@gmail.com

Sorry Romain, I don't know if I understood you. Do you have a similar example somewhere?

On 08/10/2018 14:19, Romain Manni-Bucau wrote:
Well the config point is tomee will still fork this bean cause it lust also respect tomee config so id make all services either implicitly looked up or injected in the producer and user has its own config properly instead of creating a module misbehaving OOTB in 90% of the case and needinh another piece of vonfig outside the app.

Gain sounds quite low vs the cost in that perspective.

Le lun. 8 oct. 2018 16:02, Bruno Baptista <[hidden email]> a écrit :

Thanks, will play with the module and add tests.

Cheers

On 08/10/2018 12:41, John D. Ament wrote:
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_







Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

Roberto Cortez
In reply to this post by Romain Manni-Bucau
I would prefer to have a single module is possible.

What I understood from Romain suggestion is to have a specific config that allows us to figure out what is the environment and then inject / lookup the right beans, is that correct?

On 8 Oct 2018, at 14:19, Romain Manni-Bucau <[hidden email]> wrote:

Well the config point is tomee will still fork this bean cause it lust also respect tomee config so id make all services either implicitly looked up or injected in the producer and user has its own config properly instead of creating a module misbehaving OOTB in 90% of the case and needinh another piece of vonfig outside the app.

Gain sounds quite low vs the cost in that perspective.

Le lun. 8 oct. 2018 16:02, Bruno Baptista <[hidden email]> a écrit :

Thanks, will play with the module and add tests.

Cheers

On 08/10/2018 12:41, John D. Ament wrote:
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_







Reply | Threaded
Open this post in threaded view
|

Re: Managed connection pool on Safegard

Romain Manni-Bucau
Not really, my proposal is to not change safeguard - except to simplify the injection, like having a bean with all services instead of requiring N services for the manager instantiation.

Then the integration layer is the server where it is hosted - TomEE for instance.

In any case it is owned by the integration and safeguard exposes the API to let the user extends it so I guess safeguard cant do better. I safeguard provides an EE integration it will need a new config which is more work to find and understand by the user/integration than just writing it - and trust me on that with my tomee experience ;).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le lun. 8 oct. 2018 à 16:19, Roberto Cortez <[hidden email]> a écrit :
I would prefer to have a single module is possible.

What I understood from Romain suggestion is to have a specific config that allows us to figure out what is the environment and then inject / lookup the right beans, is that correct?

On 8 Oct 2018, at 14:19, Romain Manni-Bucau <[hidden email]> wrote:

Well the config point is tomee will still fork this bean cause it lust also respect tomee config so id make all services either implicitly looked up or injected in the producer and user has its own config properly instead of creating a module misbehaving OOTB in 90% of the case and needinh another piece of vonfig outside the app.

Gain sounds quite low vs the cost in that perspective.

Le lun. 8 oct. 2018 16:02, Bruno Baptista <[hidden email]> a écrit :

Thanks, will play with the module and add tests.

Cheers

On 08/10/2018 12:41, John D. Ament wrote:
Agreed, do you want to add that module?  And if so you can add a priority annotation to enable it by default.  Would then also be good to add a test in an app server then.

John


On Mon, Oct 8, 2018, 06:46 Bruno Baptista <[hidden email]> wrote:

Hi,

I've updated the PR: https://github.com/apache/geronimo-safeguard/pull/2

Now... we should probably change the project structure and have an impl artifact for Java SE and another one for the enterprise edition, using the different ExecutionManagerProvider implementations.

What do you guys think?

Cheers

On 03/10/2018 18:53, Bruno Baptista wrote:

Thanks John and Romain,

Will work on the new FailsafeExecutionManagerProvider.

Cheers

On 03/10/2018 18:21, John D. Ament wrote:
Hi Bruno

Thanks for the PR!

I think my intention for what's provided in Safeguard is that we have an overideable per container integration that allows you to look up the executor.  So rather than having boolean logic, you use a new implementation of FailsafeExecutionManagerProvider (perhaps as an alternative).  This way the lookup can be done based on how your platform is developed.

Thanks,

John

On Wed, Oct 3, 2018 at 5:53 AM Romain Manni-Bucau <[hidden email]> wrote:
yes, this is why I mentionned to make the pool configurable to make it work in both environment and in multiple apps with different pool.

Out of my head I thought about making it injectable instead of trying all possible strategies/relying on a system properties but I just realized that we already support SE and EE with managed pool, just make a @Specializes of FailsafeExecutionManagerProvider producer.

It sounds to me more flexible and easier to understand.

wdyt?


Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 11:39, Bruno Baptista <[hidden email]> a écrit :

Hi Romain,

I've updated the PR to get the resource location from a property.
In relation to the try/catch... I'm assuming that the library is supposed to work with both SE and EE environments, if we don't catch the exception this will never work on SE. In any case, if an error exists, it will be printed and can be found.

Cheers

On 03/10/2018 10:10, Romain Manni-Bucau wrote:
Hi Bruno,

Technically this pool does not "have to be" managed ;).

That said this is a good feature. Can you make the pool configurable instead of hardcoding the default pool which is never used except in tests? Will also avoid to catch and silently ignore the error (can be an issue in servers).

Romain Manni-Bucau
@rmannibucau |  Blog | Old BlogGithub | LinkedIn | Book


Le mer. 3 oct. 2018 à 10:58, Bruno Baptista <[hidden email]> a écrit :
Hi folks,

Safegard creates a java se connection pool to handle the bulckhead and
async operations. When deployed in a container, that pool has to be
managed.

I created a PR that allows to retrieve that managed pool, if available:

https://github.com/apache/geronimo-safeguard/pull/2

Can someone please take a look at it?

Regards

--
Bruno Baptista
http://twitter.com/brunobat_