Re: [configuration] Reloading properties does not work as expected

[Oliver Heger]

Hi,

Am 30.01.2018 um 10:18 schrieb Paul Wellner Bou:
> Good morning,
> 
> i am trying to implement a reloadable property using commons-reloading2
> (2.2), without success. I tried to follow the (outdated) examples on
> https://commons.apache.org/proper/commons-configuration/userguide/howto_reloading.html#Reloading_File-based_Configurations
> (both of them).

Could you be a bit more specific about what is outdated, so that we can
update the examples?

> 
> Now I created a unit test against this API which I would expect it to pass,
> but it doesn't:
> https://gist.github.com/paulwellnerbou/dfed371d67e2f19a699b248ebf5c62d7
> 
> Any idea what I am doing wrong?

I had a deeper look at the test and the implementation, and here is what
I found out:

The class for checking whether a reload of a file is required is
FileHandlerReloadingDetector. The class records the time of the last
check and has a refreshDelay property; it only checks again after the
time configured for the delay is elapsed. The default refreshDelay is 5
seconds. This is one reason why your test does not pass; you need to
decrease this delay or wait longer.

The other reason is that there is indeed a problem in the implementation
that causes the first call to getConfiguration() to get missed by the
reload checker. The CONFIGURATION_REQUEST event is already fired before
the file to be loaded is properly initialized, and therefore, the
checker cannot set its last check time. This is initialized only at the
second call to getConfiguration(). So the test would only be successful
if getConfiguration() was called another time (taking the refresh delay
into account).

This can be considered a bug, but is probably not very problematic in
practice when the configuration is accessed on a regular basis.

HTH
Oliver

> 
> Thank you and kind regards
> Paul.
> 

-
To unsubscribe, e-mail: user-unsubscr...@commons.apache.org
For additional commands, e-mail: user-h...@commons.apache.org

Re: [POOL] EvictionTimer daemon thread

[Gary Gregory]

On Wed, Jan 31, 2018 at 1:49 AM, Mark Thomas  wrote:

> On 31/01/18 08:15, Bruno P. Kinoshita wrote:
> > Not sure if it was intentional.
> >
> > But here's the reason: https://github.com/apache/commons-pool/commit/
> 4a20cdca923bd342360f821d7020538e985d9ec2#diff-
> 38e254894b87bdf9a1758778c7ffd50fL167
> >
> > Instead of a `new Timer("", /* isDaemon */ true)`, now we have an
> implementation of `ThreadFactory` that when it creates new `Thread`s, it
> doesn't set the `setDaemon(true)`. So it just creates a thread with default
> behaviour of daemon set to false.
> >
> > As the previous behaviour was to have the threads as daemon, and there
> doesn't seem to have any arguments for dropping it, we could raise a new
> issue, with a patch, and ping Mark to see what he thinks?
>
> There is a typo in the commit message. The issue is POOL-315. It looks
> like I had finger trouble that day. I've spotted another typo in the
> changelog which I have now fixed.
>
> I don't recall any discussion of whether or not the threads should be
> daemon threads.
>
> Starting with a clean slate, I'd turn this around. What is the argument
> that the threads should be daemon threads?
>
> If the pool is correctly shutdown it should not matter as the evictor
> thread will be stopped at shutdown.
>

Are we sure that our shutdown logic always cleans up the thread-pool no
matter what?

Gary


>
> The only reason I can see for changing back to using a daemon thread is
> that it used to be a daemon thread and that allowed pools to be used
> without shutting them down cleanly.
>
> Overall, I guess I am neutral on changing it.
>
> Mark
>
>
> >
> > Cheers
> > Bruno
> >
> > (ps: the threads have a typo in their names, but it has been fixed in
> the master branch already)
> >
> >
> > 
> > From: "Wegrzyn, Jakub" 
> > To: Commons Users List 
> > Sent: Wednesday, 31 January 2018 8:54 PM
> > Subject: RE: [POOL] EvictionTimer daemon thread
> >
> >
> >
> > I couldn’t find it either.
> > Pool-351 was a commit message.
> > https://git-wip-us.apache.org/repos/asf?p=commons-pool.git;a=commit;h=
> 4a20cdca923bd342360f821d7020538e985d9ec2
> >
> > Jakub
> >
> >
> > -Original Message-
> > From: Gary Gregory [mailto:garydgreg...@gmail.com]
> > Sent: Wednesday, January 31, 2018 8:28 AM
> > To: Commons Users List 
> > Subject: Re: [POOL] EvictionTimer daemon thread
> >
> > I cannot find a POOL-351 issue. Can you double check please?
> >
> > Gary
> >
> > On Jan 31, 2018 00:15, "Wegrzyn, Jakub"  wrote:
> >
> >> Hi,
> >>
> >> We want to upgrade dbcp2 from version 2.1.1 to version 2.2.0. It
> >> requires upgrading commons-pool2 from version 2.4.2 to 2.5.0. However,
> >> during the upgrade we encountered a problem.
> >> It seems that commons-pool2 Evictor thread has been changed from
> >> daemon to non-daemon thread (Issue POOL-351, commit:
> >> 4a20cdca923bd342360f821d702053 8e985d9ec2).
> >>
> >> We cannot find any documentation describing the reason or the change
> >> itself. Can you provide more insight why that was changed and add it
> >> to the changes-report.
> >>
> >> Best regards,
> >> Jakub
> >>
> >
> > -
> > To unsubscribe, e-mail: user-unsubscr...@commons.apache.org
> > For additional commands, e-mail: user-h...@commons.apache.org
> >
>
>
> -
> To unsubscribe, e-mail: user-unsubscr...@commons.apache.org
> For additional commands, e-mail: user-h...@commons.apache.org
>
>

Re: [POOL] EvictionTimer daemon thread

[Mark Thomas]

On 31/01/18 08:15, Bruno P. Kinoshita wrote:
> Not sure if it was intentional.
> 
> But here's the reason: 
> https://github.com/apache/commons-pool/commit/4a20cdca923bd342360f821d7020538e985d9ec2#diff-38e254894b87bdf9a1758778c7ffd50fL167
> 
> Instead of a `new Timer("", /* isDaemon */ true)`, now we have an 
> implementation of `ThreadFactory` that when it creates new `Thread`s, it 
> doesn't set the `setDaemon(true)`. So it just creates a thread with default 
> behaviour of daemon set to false.
> 
> As the previous behaviour was to have the threads as daemon, and there 
> doesn't seem to have any arguments for dropping it, we could raise a new 
> issue, with a patch, and ping Mark to see what he thinks?

There is a typo in the commit message. The issue is POOL-315. It looks
like I had finger trouble that day. I've spotted another typo in the
changelog which I have now fixed.

I don't recall any discussion of whether or not the threads should be
daemon threads.

Starting with a clean slate, I'd turn this around. What is the argument
that the threads should be daemon threads?

If the pool is correctly shutdown it should not matter as the evictor
thread will be stopped at shutdown.

The only reason I can see for changing back to using a daemon thread is
that it used to be a daemon thread and that allowed pools to be used
without shutting them down cleanly.

Overall, I guess I am neutral on changing it.

Mark


> 
> Cheers
> Bruno
> 
> (ps: the threads have a typo in their names, but it has been fixed in the 
> master branch already)
> 
> 
> 
> From: "Wegrzyn, Jakub" 
> To: Commons Users List  
> Sent: Wednesday, 31 January 2018 8:54 PM
> Subject: RE: [POOL] EvictionTimer daemon thread
> 
> 
> 
> I couldn’t find it either.
> Pool-351 was a commit message. 
> https://git-wip-us.apache.org/repos/asf?p=commons-pool.git;a=commit;h=4a20cdca923bd342360f821d7020538e985d9ec2
> 
> Jakub 
> 
> 
> -Original Message-
> From: Gary Gregory [mailto:garydgreg...@gmail.com] 
> Sent: Wednesday, January 31, 2018 8:28 AM
> To: Commons Users List 
> Subject: Re: [POOL] EvictionTimer daemon thread
> 
> I cannot find a POOL-351 issue. Can you double check please?
> 
> Gary
> 
> On Jan 31, 2018 00:15, "Wegrzyn, Jakub"  wrote:
> 
>> Hi,
>>
>> We want to upgrade dbcp2 from version 2.1.1 to version 2.2.0. It 
>> requires upgrading commons-pool2 from version 2.4.2 to 2.5.0. However, 
>> during the upgrade we encountered a problem.
>> It seems that commons-pool2 Evictor thread has been changed from 
>> daemon to non-daemon thread (Issue POOL-351, commit: 
>> 4a20cdca923bd342360f821d702053 8e985d9ec2).
>>
>> We cannot find any documentation describing the reason or the change 
>> itself. Can you provide more insight why that was changed and add it 
>> to the changes-report.
>>
>> Best regards,
>> Jakub
>>
> 
> -
> To unsubscribe, e-mail: user-unsubscr...@commons.apache.org
> For additional commands, e-mail: user-h...@commons.apache.org
> 


-
To unsubscribe, e-mail: user-unsubscr...@commons.apache.org
For additional commands, e-mail: user-h...@commons.apache.org

Re: [POOL] EvictionTimer daemon thread

[Bruno P. Kinoshita]

Not sure if it was intentional.

But here's the reason: 
https://github.com/apache/commons-pool/commit/4a20cdca923bd342360f821d7020538e985d9ec2#diff-38e254894b87bdf9a1758778c7ffd50fL167

Instead of a `new Timer("", /* isDaemon */ true)`, now we have an 
implementation of `ThreadFactory` that when it creates new `Thread`s, it 
doesn't set the `setDaemon(true)`. So it just creates a thread with default 
behaviour of daemon set to false.

As the previous behaviour was to have the threads as daemon, and there doesn't 
seem to have any arguments for dropping it, we could raise a new issue, with a 
patch, and ping Mark to see what he thinks?

Cheers
Bruno

(ps: the threads have a typo in their names, but it has been fixed in the 
master branch already)



From: "Wegrzyn, Jakub" 
To: Commons Users List  
Sent: Wednesday, 31 January 2018 8:54 PM
Subject: RE: [POOL] EvictionTimer daemon thread



I couldn’t find it either.
Pool-351 was a commit message. 
https://git-wip-us.apache.org/repos/asf?p=commons-pool.git;a=commit;h=4a20cdca923bd342360f821d7020538e985d9ec2

Jakub 


-Original Message-
From: Gary Gregory [mailto:garydgreg...@gmail.com] 
Sent: Wednesday, January 31, 2018 8:28 AM
To: Commons Users List 
Subject: Re: [POOL] EvictionTimer daemon thread

I cannot find a POOL-351 issue. Can you double check please?

Gary

On Jan 31, 2018 00:15, "Wegrzyn, Jakub"  wrote:

> Hi,
>
> We want to upgrade dbcp2 from version 2.1.1 to version 2.2.0. It 
> requires upgrading commons-pool2 from version 2.4.2 to 2.5.0. However, 
> during the upgrade we encountered a problem.
> It seems that commons-pool2 Evictor thread has been changed from 
> daemon to non-daemon thread (Issue POOL-351, commit: 
> 4a20cdca923bd342360f821d702053 8e985d9ec2).
>
> We cannot find any documentation describing the reason or the change 
> itself. Can you provide more insight why that was changed and add it 
> to the changes-report.
>
> Best regards,
> Jakub
>
B�CB��[��X��ܚX�KK[XZ[�\�\�][��X��ܚX�P��[[ۜ˘\X�K�ܙ�B��܈Y][ۘ[��[X[��K[XZ[�\�\�Z[��[[ۜ˘\X�K�ܙ�B�

-
To unsubscribe, e-mail: user-unsubscr...@commons.apache.org
For additional commands, e-mail: user-h...@commons.apache.org