osdir.com

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [VOTE] [CANCEL] Release Apache Commons Pool2 2.6.1


To clarify why this change was done:
This change (putting a new item back to the idle pool was needed to prevent a dead-pool which caused an efective shutdown of all the server by staling the pool in various cases.

This solved my problem. I created test to make sure to not introduce new ones. But the code is way too much organically grown to be 100% sure. it probably need a clean rewrite.
I had another solution which created too many pooled instances. My hope was that the create() method will prevent exactly that! If this doesn't work.
Yes, we need to have a null-check on the return param of create and deal with that case. But if we still get too many idle objects, then create() is broken as well, isn't?

LieGrue,
strub



> Am 19.11.2018 um 23:34 schrieb Gary Gregory <garydgregory@xxxxxxxxx>:
> 
> 
> diff --git
> a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> index 0575f7e..6d81dbc 100644
> --- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> +++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> @@ -920,8 +920,7 @@
>             // In case there are already threads waiting on something in
> the pool
>             // (e.g. idleObjects.takeFirst(); then we need to provide them
> a fresh instance.
>             // Otherwise they will be stuck forever (or until timeout)
> -            PooledObject<T> freshPooled = create();
> -            idleObjects.put(freshPooled);
> +            addObject();
>         }
>     }
> 
> But causes a failure:
> 
> [INFO] Running org.apache.commons.pool2.impl.TestAbandonedObjectPool
> [ERROR] Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed:
> 12.253 s <<< FAILURE! - in
> org.apache.commons.pool2.impl.TestAbandonedObjectPool
> [ERROR]
> testAbandonedInvalidate(org.apache.commons.pool2.impl.TestAbandonedObjectPool)
> Time elapsed: 3.668 s  <<< FAILURE!
> java.lang.AssertionError: expected:<5> but was:<4>
>        at
> org.apache.commons.pool2.impl.TestAbandonedObjectPool.testAbandonedInvalidate(TestAbandonedObjectPool.java:202)
> 
> Maybe this is due to my busy CPU, not sure.
> 
> Gary
> 
> 
>> 
>> Phil
>> 
>> On 11/19/18 2:31 PM, Gary Gregory wrote:
>>> A unit test? Yes please! :-)
>>> 
>>> Gary
>>> 
>>> On Mon, Nov 19, 2018 at 1:23 PM Mark Struberg <struberg@xxxxxxxx.invalid
>>> 
>>> wrote:
>>> 
>>>> +1 for the null check.
>>>> 
>>>> Do you want to re-open the ticket and create a patch?
>>>> 
>>>> I've created a unit test which proves my original problem with the
>>>> dead-lock.
>>>> So any improvement should be rather on the safe side from here on.
>>>> 
>>>> 
>>>> Regarding the RC: this is really not needed anymore when working with
>> GIT
>>>> as nothing gets pushed/released to the main repository! See the config
>>>> changes I did to the maven-release-plugin.
>>>> 
>>>> txs and LieGrue,
>>>> strub
>>>> 
>>>> 
>>>> 
>>>>> Am 19.11.2018 um 16:43 schrieb Phil Steitz <phil.steitz@xxxxxxxxx>:
>>>>> 
>>>>> On 11/19/18 8:19 AM, Gary Gregory wrote:
>>>>>> On Mon, Nov 19, 2018 at 6:04 AM Rob Tompkins <chtompki@xxxxxxxxx>
>>>> wrote:
>>>>>>> I’d be happy to roll the release if we get master to where you want
>>>> it.
>>>>>> IMO, we should integrate the recent PR I mentioned and roll RC3. Note
>>>> that this vote subject thread did not contain an RC number. Sticking to
>> the
>>>> usual process would be less troublesome IMO.
>>>>> I have not had a chance to fully review and am not really active in
>>>> [pool] any more, but I did notice that the fix for POOL-356 is missing a
>>>> null check between these two added statements:
>>>>>  PooledObject<T> freshPooled = create();
>>>>> idleObjects.put(freshPooled);
>>>>> 
>>>>> create() can return null and while in general it won't in this
>>>> activation context, given the lack of sync control, it is possible that
>> a
>>>> return hits between the if test and execution resulting in no capacity
>> to
>>>> create.
>>>>> I also notice some system.outs made it into the test code in one of the
>>>> commits related to POOL-340.
>>>>> Phil
>>>>>> Gary
>>>>>> 
>>>>>> 
>>>>>>> Cheers,
>>>>>>> -Rob
>>>>>>> 
>>>>>>>> On Nov 19, 2018, at 7:18 AM, Mark Struberg
>> <struberg@xxxxxxxx.INVALID
>>>>>>> wrote:
>>>>>>>> Oki, I now see what you mean.
>>>>>>>> 
>>>>>>>> We actually have 3 source zips now.
>>>>>>>> 
>>>>>>>> .src.zip
>>>>>>>> .source-release.zip
>>>>>>>> src.jar
>>>>>>>> 
>>>>>>>> That's a mess.
>>>>>>>> 
>>>>>>>> There should only be 2:
>>>>>>>> * source-release.zip is the official ASF packages whole build
>> sources.
>>>>>>> This includes the pom, build structure etc.
>>>>>>>> * src.jar is the sources which are automatically downloaded by the
>>>> IDEs
>>>>>>> for debugging purpose.
>>>>>>>> We have both of them because commons-pool2 is a single-module
>> project.
>>>>>>>> And yes, we need both of them. What we do not need is the src.zip. I
>>>>>>> have no clue yet where this comes from but it shouldn't be here.
>>>>>>>> The good news:
>>>>>>>> By leveraging native GIT we now can simply a.) drop the maven
>> stating
>>>>>>> repo in repository.a.o and b.) drop the release branch and tag from
>> my
>>>>>>> github account and re-roll the release without any weird RC hacks.
>>>>>>>> Will do that,
>>>>>>>> * fix the maven setup
>>>>>>>> * happy to also include the new ticket
>>>>>>>> * re-roll the release this afternoon.
>>>>>>>> 
>>>>>>>> LieGrue,
>>>>>>>> strub
>>>>>>>> 
>>>>>>>>> Am 16.11.2018 um 23:10 schrieb Romain Manni-Bucau <
>>>>>>> rmannibucau@xxxxxxxxx>:
>>>>>>>>> Le ven. 16 nov. 2018 22:54, Gary Gregory <garydgregory@xxxxxxxxx>
>> a
>>>>>>> écrit :
>>>>>>>>>> On Fri, Nov 16, 2018 at 2:32 PM Romain Manni-Bucau <
>>>>>>> rmannibucau@xxxxxxxxx>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> Le ven. 16 nov. 2018 21:23, Gary Gregory <garydgregory@xxxxxxxxx
>>> 
>>>> a
>>>>>>>>>> écrit
>>>>>>>>>>> :
>>>>>>>>>>> 
>>>>>>>>>>>> On Wed, Nov 14, 2018 at 8:59 AM Mark Struberg
>>>>>>>>>> <struberg@xxxxxxxx.invalid
>>>>>>>>>>>> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>>> Oki, now the full VOTE text!
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I'd like to call a VOTE on releasing Apache Commons pool2 2.6.1
>>>>>>>>>>>>> The release was run with JDK-1.7 to ensure Java7 compatibility.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> The ASF staging repository is at
>>>>>>>>>>>>> 
>>>> 
>> https://repository.apache.org/content/repositories/orgapachecommons-1396/
>>>>>>>>>>>>> The source zip is at
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>> 
>> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/
>>>>>>>>>>>>> The sha1 of the source-release zip is
>>>>>>>>>>>>> 17b01d1e776b7e2b9987b665e1b4e456c02ffa1c
>>>>>>>>>>>>> The sha512 is
>>>>>>>>>>>>> 
>>>> 
>> 982275c963c09e11dd38a3b6621f2a67bab42b6744a1629ab97b7323208b31730b756a7d5bc6dabee54ba0e9f72c8296904f36919fd421fee8e59786c587c388
>>>>>>>>>>>> For me:
>>>>>>>>>>>> 
>>>>>>>>>>>> $ sha512sum commons-pool2-2.6.1-src.zip
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>> 
>> 2b95b00a22bf72a7cdf77f2e40796d126b4a0d7b669564b8b04cd0c884252acd3dac356fe55a9fdaadd4767e13eef560995989cb2d39f862f8d3b7e1d06c773e
>>>>>>>>>>>> *commons-pool2-2.6.1-src.zip
>>>>>>>>>>>> 
>>>>>>>>>>>> Which is not what you list above. Please advise.
>>>>>>>>>>>> 
>>>>>>>>>>> Src vs source-release?
>>>>>>>>>>> 
>>>>>>>>>> That's the problem with inventing a new release process... why do
>> we
>>>>>>> have
>>>>>>>>>> BOTH:
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>> 
>> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/commons-pool2-2.6.1-src.zip
>>>>>>>>>> AND
>>>>>>>>>> 
>>>>>>>>>> 
>>>> 
>> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/commons-pool2-2.6.1-source-release.zip
>>>>>>>>>> And more importantly why are they _different_? Which one will be
>>>> used
>>>>>>> in
>>>>>>>>>> the dist/release area?
>>>>>>>>>> 
>>>>>>>>> Looks like pool didnt do its homework and kept the old assembly
>>>> (src),
>>>>>>>>> source-release comes from the parent and is likely the one to keep
>>>> IMHO
>>>>>>>>> 
>>>>>>>>>> Gary
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>> Gary
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>>> I added my KEY (struberg at apache.org) to our dist KEYS file
>>>>>>>>>>>>> https://dist.apache.org/repos/dist/release/commons/KEYS
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I've created the release in a GIT manner and pushed the
>> according
>>>>>>>>>>> changes
>>>>>>>>>>>>> to my ASF-linked github repo
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>> https://github.com/struberg/commons-pool/tree/release_branch_2.6.1
>>>>>>>>>>>>> the sha1 of the commit is
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>> 
>> https://github.com/struberg/commons-pool/commit/c910171d9d8c8f5f895b7d18381fc03a51b2a019
>>>>>>>>>>>>> the tag is
>>>>>>>>>>>>> 
>>>> https://github.com/struberg/commons-pool/tree/commons-pool2-2.6.1
>>>>>>>>>>>>> c910171
>>>>>>>>>>>>> <
>>>> 
>> https://github.com/struberg/commons-pool/tree/commons-pool2-2.6.1c910171
>>>>>>>>>>>>> This will get pushed to the ASF cannonical repo once the VOTE
>>>>>>>>>> succeeds.
>>>>>>>>>>>>> Site will be updated once the release has passed.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Please VOTE:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> [+1] go ship it!
>>>>>>>>>>>>> [+0] meh, I don't care
>>>>>>>>>>>>> [-1] stop there is a ${showstopper} (that means something
>>>>>>> _important_
>>>>>>>>>>> is
>>>>>>>>>>>>> missing!)
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Here is my own +1
>>>>>>>>>>>>> checked:
>>>>>>>>>>>>> * signature
>>>>>>>>>>>>> * hashes
>>>>>>>>>>>>> * LICENSE
>>>>>>>>>>>>> * NOTICE
>>>>>>>>>>>>> * rat
>>>>>>>>>>>>> * builds fine with various JDKs
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> LieGrue,
>>>>>>>>>>>>> strub
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Am 14.11.2018 um 10:13 schrieb Mark Struberg
>>>>>>>>>>> <struberg@xxxxxxxx.INVALID
>>>>>>>>>>>>>> :
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> PS: I've created the release in a GIT manner and pushed the
>>>>>>>>>> according
>>>>>>>>>>>>> changes to my ASF-linked github repo
>>>> https://github.com/struberg/commons-pool/tree/release_branch_2.6.1
>>>>>>>>>>>>>> the sha1 of the commit is
>>>>>>>>>>>>>> 
>>>> 
>> https://github.com/struberg/commons-pool/commit/c910171d9d8c8f5f895b7d18381fc03a51b2a019
>>>>>>>>>>>>>> the tag is
>>>>>>>>>>>>>> 
>>>> https://github.com/struberg/commons-pool/tree/commons-pool2-2.6.1
>>>>>>>>>>>>>> c910171
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> This will get pushed to the ASF cannonical repo once the VOTE
>>>>>>>>>>> succeeds.
>>>>>>>>>>>>>> Yay, this is the way GIT works and before someone not familiar
>>>> with
>>>>>>>>>>> GIT
>>>>>>>>>>>>> screams that this is not hosted on ASF: This got discussed on
>> the
>>>>>>>>>> board
>>>>>>>>>>>>> level a long time ago (when we did DeltaSpike and CouchDB as
>> the
>>>>>>> very
>>>>>>>>>>>> first
>>>>>>>>>>>>> GIT repos at the ASF) and is perfectly fine as all this is
>> based
>>>> on
>>>>>>>>>>>>> cryptographically strong steps.
>>>>>>>>>>>>>> LieGrue,
>>>>>>>>>>>>>> strub
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Am 14.11.2018 um 09:17 schrieb Mark Struberg
>>>>>>>>>>>> <struberg@xxxxxxxx.INVALID
>>>>>>>>>>>>>> :
>>>>>>>>>>>>>>> Hi folks!
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> I'm currently preparing the release for commons-pool2-2.6.1
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> So far I did
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> * fix the missing parts in changes.xml
>>>>>>>>>>>>>>> * generate + copy the RELEASE_NOTES
>>>>>>>>>>>>>>> * run the maven release (after fixing the setup...)
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> The ASF staging repository is at
>>>>>>>>>>>>>>> 
>>>> 
>> https://repository.apache.org/content/repositories/orgapachecommons-1396/
>>>>>>>>>>>>>>> The source zip is at
>>>>>>>>>>>>>>> 
>>>> 
>> https://repository.apache.org/content/repositories/orgapachecommons-1396/org/apache/commons/commons-pool2/2.6.1/
>>>>>>>>>>>>>>> The sha1 of the source-release zip is
>>>>>>>>>>>>> 17b01d1e776b7e2b9987b665e1b4e456c02ffa1c
>>>>>>>>>>>>>>> The sha512 is
>>>> 
>> 982275c963c09e11dd38a3b6621f2a67bab42b6744a1629ab97b7323208b31730b756a7d5bc6dabee54ba0e9f72c8296904f36919fd421fee8e59786c587c388
>>>>>>>>>>>>>>> I added my KEY (struberg at apache.org) to our dist KEYS
>> file
>>>>>>>>>>>>>>> https://dist.apache.org/repos/dist/release/commons/KEYS
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> I will now continue with the follow up steps and then call an
>>>>>>>>>>> official
>>>>>>>>>>>>> VOTE.
>>>>>>>>>>>>>>> Please let me know if something went wrong so far!
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> LieGrue,
>>>>>>>>>>>>>>> strub
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>> ---------------------------------------------------------------------
>>>>>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>>>>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>> ---------------------------------------------------------------------
>>>>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>>>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>> ---------------------------------------------------------------------
>>>>>>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>>>>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>>>>>> 
>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>>>>>> 
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>>>>> 
>>>>>>> 
>>>>> 
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>> 
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>>>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>>>> 
>>>> 
>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
>> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
>> 
>> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx