Hi Romain,

About "The point is not the cdi bean but the executor. So high level you deploy an
app not using safeguard but it being present and you ensure the container
has no executor resource instantiated (you will get one (the facade))."

Sorry Romain, I still don't understand how the code in the PR can possible affect something not using the FT API or Safeguard in particular.

In relation to the Managed executor... What you say makes sense but I wonder how likely it is to happen and if it's enough to hold the PR. Do you have a custom executor example somewhere?

Cheers

Bruno Baptista
https://twitter.com/brunobat_


On 23/11/18 15:14, Romain Manni-Bucau wrote:
Le ven. 23 nov. 2018 à 15:49, Bruno Baptista <bruno...@gmail.com> a écrit :

Hi Romain,

Thanks for your comment.

The class doing the resource injection is lazy loaded, specifically
/FailsafeContainerExecutionManagerProvider/. I did verify it in
development but no test was produced... And to say the truth I wouldn't
know how to validate if a bean has already been loaded or not. Can you
please provide a test example?

The point is not the cdi bean but the executor. So high level you deploy an
app not using safeguard but it being present and you ensure the container
has no executor resource instantiated (you will get one (the facade)).


Please explain what do you mean by "MP-fault-tolerance executor for that
case if noone exists". It will exist, that's the whole purpose of this
PR. Can you please provide an example where a
/ManagedScheduledExecutorService/ will not be present?

You can see it as "don't let it default to a random executor". This is the
current behavior. So here is what can happen:

1. The user doesnt use any executor -> it defaults -> it is ok
2. The user uses one or more executors for his app -> it defaults to it ->
it messes up the app and does not have the expected setting

Case 2 is important cause it can really make it not functional and even
lead to locks in some cases so better to not let it happen and just create
a safeguard executor if
the user didnt specify he wants safeguard to use the executor
"'mysafeguardexecutor".

This is why the config is important and I mentionned it early even if it is
not the most sexy part to do, I agree.


Cheers

Bruno Baptista
https://twitter.com/brunobat_


On 23/11/18 14:39, Romain Manni-Bucau wrote:
It's lazily loaded, so no worries on that regard.
What is "it" here? :)

Conretely the bean instantiation yes cause it is normal scoped and the
resource too cause it is by default lazy in tomee (service-jar.xml) but
it
is worth a test that prevent regression on that behavior IMHO, I didn't
catch on in the PR.

Concretely in terms of container we can want to create a dedicated
MP-fault-tolerance executor for that case if noone exists and the user
didn't specify one cause this default behavior (cumulated with tomee
defaulting on @Resouce) will make this not reliable which is quite
ridiculous when you think about it for something about failt tolerance.
This is why it should be in before next release. Now if you do the PR
next
week it is fine, was not to do it today but to ensure it is not merged
and
the enthusiasm makes it forgotten.

Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<https://rmannibucau.metawerx.net/> | Old Blog
<http://rmannibucau.wordpress.com> | Github <
https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book
<
https://www.packtpub.com/application-development/java-ee-8-high-performance


Le ven. 23 nov. 2018 à 15:18, Jonathan Gallimore <
jonathan.gallim...@gmail.com> a écrit :

Maybe add those config options in a second PR? What do you think?

Jon

On Fri, Nov 23, 2018 at 2:01 PM Bruno Baptista <bruno...@gmail.com>
wrote:
Hi Romain,

In the end I decided to simply use the server default, for now.

It will only be used if annotations are called in the code. It's lazily
loaded, so no worries on that regard.

Cheers.

Bruno Baptista
https://twitter.com/brunobat_


On 23/11/18 12:31, Romain Manni-Bucau wrote:
Didnt you want to make the pool configurable and not instantiated if
not
used?

Le ven. 23 nov. 2018 13:20, Daniel Cunha <daniels...@apache.org> a
écrit :
Hey Bruno,

awesome! It really sounds good! I just push my +1 :)

Em sex, 23 de nov de 2018 às 06:44, Bruno Baptista <
bruno...@gmail.com>
escreveu:

Thanks!

Additionally, I've requested a Safeguard 1.0.1 release. we shouldn't
be
using snapshots.

Cheers

Bruno Baptista
https://twitter.com/brunobat_


On 22/11/18 19:30, Roberto Cortez wrote:
Cool! Thank you.

I’ll have a look.

On 22 Nov 2018, at 19:08, Bruno Baptista <bruno...@gmail.com>
wrote:
Hi!

I think the code is ready. Can some of you please review this pull
request:
https://github.com/apache/tomee/pull/201

Related to:TOMEE-2278 <
https://issues.apache.org/jira/browse/TOMEE-2278>-
Use Managed Executor with Safeguard Fault Tolerance lib
Cheers.

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


--
Daniel "soro" Cunha
https://twitter.com/dvlc_

Reply via email to