Re: [VOTE] Release Apache Commons DBCP 2.8.0 based on RC1
ping ;-) On Mon, Sep 21, 2020 at 3:34 PM Gary Gregory wrote: > We have fixed a few bugs and added some enhancements since Apache Commons > DBCP 2.7.0 was released, so I would like to release Apache Commons DBCP > 2.8.0. > > Apache Commons DBCP 2.8.0 RC1 is available for review here: > https://dist.apache.org/repos/dist/dev/commons/dbcp/2.8.0-RC1 (svn > revision 41530) > > The Git tag commons-dbcp-2.8.0-RC1 commit for this RC is > 5dea53a5e089862c37c830c725315d90b444852e which you can browse here: > > https://gitbox.apache.org/repos/asf?p=commons-dbcp.git;a=commit;h=5dea53a5e089862c37c830c725315d90b444852e > You may checkout this tag using: > git clone https://gitbox.apache.org/repos/asf/commons-dbcp.git > --branch commons-dbcp-2.8.0-RC1 commons-dbcp-2.8.0-RC1 > > Maven artifacts are here: > > https://repository.apache.org/content/repositories/orgapachecommons-1528/org/apache/commons/commons-dbcp2/2.8.0/ > > These are the artifacts and their hashes: > > #Release SHA-512s > #Mon Sep 21 15:23:26 EDT 2020 > > commons-dbcp2-2.8.0-bin.tar.gz=3f09d7eb45979d239454d4239984c920c42ba642b069d520408d613262d7f5d9b7c86e3041617bbefa460cd84f3babed7b6cde140dbfe46d95fdd733b66854f3 > > commons-dbcp2-2.8.0-bin.zip=353e47ec4c927edf14e843732e24bce1ede3782589d45fae2ccb93584db3d413ff030bbdffb1f04c76084c4b2fda7a25c24802e1ca9de559ff5e375304ba266e > > commons-dbcp2-2.8.0-javadoc.jar=28528baf0a5a3796fcd0a8cad3a040a6fc28ae8b44065ea188cf97d26eb6777c51657d6405c89457f8014d8683f51bbd601deb28aa1032850fc040f1b7ee561c > > commons-dbcp2-2.8.0-sources.jar=61d06856441a8d9147f339e354f15742df9ab701d3cfafd81b927b24313d552cd7a3268c6bcf84090987f12953014e6eb712b660886b0ab04032fa54212d93a9 > > commons-dbcp2-2.8.0-src.tar.gz=9a7d3877a83e961db77d4766c299db2fcb120a760c4d30db54faca983b97ab92e001435ce558ec7be7483b27744235fe246b4267c884add6dd0130fae78c2d6d > > commons-dbcp2-2.8.0-src.zip=81a018d18f0213525350545afc82fb58de1521fd78cdaac7439a5c5af53652c2a1f5e46bf06a8651754e26fc542b99361eda1207703929b09e43f9037376837c > > commons-dbcp2-2.8.0-test-sources.jar=255c2f456fceac57da0b215fa119a33152b0ef902e0a17b1211578dc042ec1881f8b02ffa1c144343c482ff728663090762dd806d9f4fb17c1467f2a3bd3 > > commons-dbcp2-2.8.0-tests.jar=291dc8210b11fb5cd980c94587be1db1265b4c74ced9c26c793776b2339f4a470478a4dc5fd9f7a57739d1e25a5740e4eaeada1c2c1e13b5ddc5e3ad6bdc1ae9 > > > I have tested this with: > > mvn -V -Prelease -Ptest-deploy -P jacoco -P japicmp clean package site > deploy > > Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f) > Maven home: C:\Java\apache-maven-3.6.3\bin\.. > Java version: 1.8.0_251, vendor: Oracle Corporation, runtime: C:\Program > Files\Java\jdk1.8.0_251\jre > Default locale: en_US, platform encoding: Cp1252 > OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows" > > Details of changes since 2.7.0 are in the release notes: > > https://dist.apache.org/repos/dist/dev/commons/dbcp/2.8.0-RC1/RELEASE-NOTES.txt > > https://dist.apache.org/repos/dist/dev/commons/dbcp/2.8.0-RC1/site/changes-report.html > > Site: > > https://dist.apache.org/repos/dist/dev/commons/dbcp/2.8.0-RC1/site/index.html > (note some *relative* links are broken and the 2.8.0 directories are > not yet created - these will be OK once the site is deployed.) > > JApiCmp Report (compared to 2.7.0): > > https://dist.apache.org/repos/dist/dev/commons/dbcp/2.8.0-RC1/site/japicmp.html > > RAT Report: > > https://dist.apache.org/repos/dist/dev/commons/dbcp/2.8.0-RC1/site/rat-report.html > > KEYS: > https://www.apache.org/dist/commons/KEYS > > Please review the release candidate and vote. > This vote will close no sooner than 72 hours from now. > > [ ] +1 Release these artifacts > [ ] +0 OK, but... > [ ] -0 OK, but really should fix... > [ ] -1 I oppose this release because... > > Thank you, > > Gary Gregory, > Release Manager (using key 86fdc7e2a11262cb) > > For following is intended as a helper and refresher for reviewers. > > Validating a release candidate > == > > These guidelines are NOT complete. > > Requirements: Git, Java, Maven. > > You can validate a release from a release candidate (RC) tag as follows. > > 1) Clone and checkout the RC tag > > git clone https://gitbox.apache.org/repos/asf/commons-dbcp.git --branch > commons-dbcp-2.8.0-RC1 commons-dbcp-2.8.0-RC1 > cd commons-dbcp-2.8.0-RC1 > > 2) Check Apache licenses > > This step is not required if the site includes a RAT report page which you > then must check. > > mvn apache-rat:check > > 3) Check binary compatibility > > Older components still use Apache Clirr: > > This step is not required if the site includes a Clirr report page which > you then must check. > > mvn clirr:check > > Newer components use JApiCmp with the japicmp Maven Profile: > > This step is not required if the site includes a JApiCmp report page which > you then must check. > > mvn install -DskipTests -P japicmp japicmp:cmp > > 4) Build the package > > mvn -V clean package > > You can record
Re: [dbcp][pool] Use abort instead of close for abandoned connections?
On Wed, Sep 23, 2020 at 6:31 PM Phil Steitz wrote: > > On 9/23/20 2:57 PM, Gary Gregory wrote: > > This all sounds reasonable but it would be easier to see in a PR. > > I will do that once the new version of [pool] is released. Otherwise, I > would have to add snapshot dependency, which I am sure would cause CI to > fail. > OK, sounds good, I'll push out a [pool] RC later this week after the [dbcp] RC. Gary > > Phil > > > > > Gary > > > > On Tue, Sep 22, 2020 at 8:30 PM Phil Steitz > wrote: > > > >> On 9/14/20 10:10 AM, Gary Gregory wrote: > >>> On Mon, Sep 14, 2020 at 1:07 PM Phil Steitz > >> wrote: > On 9/14/20 9:36 AM, Gary Gregory wrote: > > This feature is now in Pool master. I will prepare an RC soon if you > > all think we are good to go so we can then move on to DBCP. > I am still working on testing this in the DBCP use case. Probably best > to wait a little for others to review and for me to get the DBCP > change > tested against current pool sources. I should be able to finish that > this weekend. > >> I implemented changes in DBCP, based on recently committed changes in > >> pool. I made a few decisions that I would appreciate feedback on. > >> > >> 1. The JDBC abort method requires an Executor as actual parameter. I > >> added a FixedThreadPool executor with a max of 3 threads to > >> PoolableConnectionFactory for this purpose. Given that this operation > >> might hang sometimes, I thought it best to allow more than a single > >> thread. I guess it could be configurable, but 3 seemed a reasonable > >> choice. I copied the custom thread factory used by pool's EvictionTimer > >> to source daemon threads based on ccl for this. I then added a close() > >> method to PCF that closes the executor and modified BasicDataSource > >> close to call this. > >> > >> 2. I used p.getObject().getInnermostDelegate().abort(executor) in PCF > >> destroyObject when abandoned mode is passed in rather than just > >> getObject().abort as destroy invokes close (I wonder if that is a bug?) > >> > >> 3. I changed JdbcBridge#abort to remove the checkOpen() check. I was > >> getting exceptions in my concurrency tests where connections were being > >> closed but then considered abandoned. This is a legit race that abort > >> will generally handle fine and I don't see the need to throw. > >> > >> Phil > >> > >>> Sounds good. > >>> > >>> Gary > >>> > Phil > > > Gary > > > > On Tue, Sep 8, 2020 at 9:16 AM Gary Gregory > >> wrote: > >> FWIW, I like the name "DestroyMode" because it matches the "destroy" > >> in the method name. > >> Gary > >> > >> On Mon, Sep 7, 2020, 19:08 Gary Gregory > >> wrote: > >>> On Mon, Sep 7, 2020 at 6:02 PM Phil Steitz > >> wrote: > On 9/3/20 2:44 AM, Mark Thomas wrote: > > On 31/08/2020 01:05, Phil Steitz wrote: > > > > > > > >> If others agree it is a good idea for dbcp, I can do it. I can > >> see the > >> argument that its better to stay with close() even for abandoned > >> and I > >> have not been able to get the deadlock to happen, so I would > like > >> to > >> wait a bit and allow others to weigh in. Similarly for pool, I > >> would > >> like to get more community feedback before adding to the > >> interface. > > Hmm. > > > > On one hand if the driver deadlocks I don't see how that can be > >> anything > > other than a driver bug. If multiple code paths obtain multiple > >> locks > > then those code paths must always obtain those locks in the same > >> order. > > Anything else is a bug that is likely to result in a deadlock in > a > > multithreaded environment. > > > > On the other hand, it could be argued that the situation only > >> arises > > when an application doesn't correctly return connections to the > >> pool > > and/or keeps them for too long and/or doesn't configure the pool > > correctly for their usage pattern. > > > > The approach of adding > > > > PooledObjectFactory.destroyObject(PooledObject,CloseMode) > > > > where CloseMode is an Enum with two values looks reasonable to > me. > I have started to work on the [pool] changes for this. I want to > >> check > two things before completing the PR: > > 1. "Close" is a [dbcp] concept which does not make sense for all > >> pool > factories, so I am going to name the enum "DestroyMode" and the > two > modes, "Default" and "Abandoned". That leaves room for other > modes > >> like > "Evicted" or "Invalid" later. > > 2. Speaking of later, technically adding modes will not break > binary > compatibility. Are we going to be OK adding outside a major > >> release? > If the answer is no, I might argue to include the other
Re: [dbcp][pool] Use abort instead of close for abandoned connections?
On 9/23/20 2:57 PM, Gary Gregory wrote: This all sounds reasonable but it would be easier to see in a PR. I will do that once the new version of [pool] is released. Otherwise, I would have to add snapshot dependency, which I am sure would cause CI to fail. Phil Gary On Tue, Sep 22, 2020 at 8:30 PM Phil Steitz wrote: On 9/14/20 10:10 AM, Gary Gregory wrote: On Mon, Sep 14, 2020 at 1:07 PM Phil Steitz wrote: On 9/14/20 9:36 AM, Gary Gregory wrote: This feature is now in Pool master. I will prepare an RC soon if you all think we are good to go so we can then move on to DBCP. I am still working on testing this in the DBCP use case. Probably best to wait a little for others to review and for me to get the DBCP change tested against current pool sources. I should be able to finish that this weekend. I implemented changes in DBCP, based on recently committed changes in pool. I made a few decisions that I would appreciate feedback on. 1. The JDBC abort method requires an Executor as actual parameter. I added a FixedThreadPool executor with a max of 3 threads to PoolableConnectionFactory for this purpose. Given that this operation might hang sometimes, I thought it best to allow more than a single thread. I guess it could be configurable, but 3 seemed a reasonable choice. I copied the custom thread factory used by pool's EvictionTimer to source daemon threads based on ccl for this. I then added a close() method to PCF that closes the executor and modified BasicDataSource close to call this. 2. I used p.getObject().getInnermostDelegate().abort(executor) in PCF destroyObject when abandoned mode is passed in rather than just getObject().abort as destroy invokes close (I wonder if that is a bug?) 3. I changed JdbcBridge#abort to remove the checkOpen() check. I was getting exceptions in my concurrency tests where connections were being closed but then considered abandoned. This is a legit race that abort will generally handle fine and I don't see the need to throw. Phil Sounds good. Gary Phil Gary On Tue, Sep 8, 2020 at 9:16 AM Gary Gregory wrote: FWIW, I like the name "DestroyMode" because it matches the "destroy" in the method name. Gary On Mon, Sep 7, 2020, 19:08 Gary Gregory wrote: On Mon, Sep 7, 2020 at 6:02 PM Phil Steitz wrote: On 9/3/20 2:44 AM, Mark Thomas wrote: On 31/08/2020 01:05, Phil Steitz wrote: If others agree it is a good idea for dbcp, I can do it. I can see the argument that its better to stay with close() even for abandoned and I have not been able to get the deadlock to happen, so I would like to wait a bit and allow others to weigh in. Similarly for pool, I would like to get more community feedback before adding to the interface. Hmm. On one hand if the driver deadlocks I don't see how that can be anything other than a driver bug. If multiple code paths obtain multiple locks then those code paths must always obtain those locks in the same order. Anything else is a bug that is likely to result in a deadlock in a multithreaded environment. On the other hand, it could be argued that the situation only arises when an application doesn't correctly return connections to the pool and/or keeps them for too long and/or doesn't configure the pool correctly for their usage pattern. The approach of adding PooledObjectFactory.destroyObject(PooledObject,CloseMode) where CloseMode is an Enum with two values looks reasonable to me. I have started to work on the [pool] changes for this. I want to check two things before completing the PR: 1. "Close" is a [dbcp] concept which does not make sense for all pool factories, so I am going to name the enum "DestroyMode" and the two modes, "Default" and "Abandoned". That leaves room for other modes like "Evicted" or "Invalid" later. 2. Speaking of later, technically adding modes will not break binary compatibility. Are we going to be OK adding outside a major release? If the answer is no, I might argue to include the other natural modes now. Yes, IMO, it is OK to add enum values in a minor release since it does not break binary (or source) compatibility. Gary Phil I do agree that abort() should only be used in the case of abandoned connections. Mark - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For
Re: [dbcp][pool] Use abort instead of close for abandoned connections?
This all sounds reasonable but it would be easier to see in a PR. Gary On Tue, Sep 22, 2020 at 8:30 PM Phil Steitz wrote: > > On 9/14/20 10:10 AM, Gary Gregory wrote: > > On Mon, Sep 14, 2020 at 1:07 PM Phil Steitz > wrote: > >> > >> On 9/14/20 9:36 AM, Gary Gregory wrote: > >>> This feature is now in Pool master. I will prepare an RC soon if you > >>> all think we are good to go so we can then move on to DBCP. > >> I am still working on testing this in the DBCP use case. Probably best > >> to wait a little for others to review and for me to get the DBCP change > >> tested against current pool sources. I should be able to finish that > >> this weekend. > > I implemented changes in DBCP, based on recently committed changes in > pool. I made a few decisions that I would appreciate feedback on. > > 1. The JDBC abort method requires an Executor as actual parameter. I > added a FixedThreadPool executor with a max of 3 threads to > PoolableConnectionFactory for this purpose. Given that this operation > might hang sometimes, I thought it best to allow more than a single > thread. I guess it could be configurable, but 3 seemed a reasonable > choice. I copied the custom thread factory used by pool's EvictionTimer > to source daemon threads based on ccl for this. I then added a close() > method to PCF that closes the executor and modified BasicDataSource > close to call this. > > 2. I used p.getObject().getInnermostDelegate().abort(executor) in PCF > destroyObject when abandoned mode is passed in rather than just > getObject().abort as destroy invokes close (I wonder if that is a bug?) > > 3. I changed JdbcBridge#abort to remove the checkOpen() check. I was > getting exceptions in my concurrency tests where connections were being > closed but then considered abandoned. This is a legit race that abort > will generally handle fine and I don't see the need to throw. > > Phil > > > Sounds good. > > > > Gary > > > >> Phil > >> > >>> Gary > >>> > >>> On Tue, Sep 8, 2020 at 9:16 AM Gary Gregory > wrote: > FWIW, I like the name "DestroyMode" because it matches the "destroy" > in the method name. > > Gary > > On Mon, Sep 7, 2020, 19:08 Gary Gregory > wrote: > > On Mon, Sep 7, 2020 at 6:02 PM Phil Steitz > wrote: > >> On 9/3/20 2:44 AM, Mark Thomas wrote: > >>> On 31/08/2020 01:05, Phil Steitz wrote: > >>> > >>> > >>> > If others agree it is a good idea for dbcp, I can do it. I can > see the > argument that its better to stay with close() even for abandoned > and I > have not been able to get the deadlock to happen, so I would like > to > wait a bit and allow others to weigh in. Similarly for pool, I > would > like to get more community feedback before adding to the > interface. > >>> Hmm. > >>> > >>> On one hand if the driver deadlocks I don't see how that can be > anything > >>> other than a driver bug. If multiple code paths obtain multiple > locks > >>> then those code paths must always obtain those locks in the same > order. > >>> Anything else is a bug that is likely to result in a deadlock in a > >>> multithreaded environment. > >>> > >>> On the other hand, it could be argued that the situation only > arises > >>> when an application doesn't correctly return connections to the > pool > >>> and/or keeps them for too long and/or doesn't configure the pool > >>> correctly for their usage pattern. > >>> > >>> The approach of adding > >>> > >>> PooledObjectFactory.destroyObject(PooledObject,CloseMode) > >>> > >>> where CloseMode is an Enum with two values looks reasonable to me. > >> I have started to work on the [pool] changes for this. I want to > check > >> two things before completing the PR: > >> > >> 1. "Close" is a [dbcp] concept which does not make sense for all > pool > >> factories, so I am going to name the enum "DestroyMode" and the two > >> modes, "Default" and "Abandoned". That leaves room for other modes > like > >> "Evicted" or "Invalid" later. > >> > >> 2. Speaking of later, technically adding modes will not break binary > >> compatibility. Are we going to be OK adding outside a major > release? > >> If the answer is no, I might argue to include the other natural > modes now. > > Yes, IMO, it is OK to add enum values in a minor release since it > does > > not break binary (or source) compatibility. > > > > Gary > > > >> Phil > >> > >>> I do agree that abort() should only be used in the case of > abandoned > >>> connections. > >>> > >>> Mark > >>> > >>> > - > >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > >>> For additional commands, e-mail: dev-h...@commons.apache.org > >>> > >> >
[io] FileUtils.isFileNewer/Older uses LocalTime.now() not LocalTime.MIDNIGHT
The FileUtils uses LocalTime.now() in isFileNewer/Older, for example:
public static boolean isFileNewer(final File file, final ChronoLocalDate
chronoLocalDate) {
return isFileNewer(file, chronoLocalDate, LocalTime.now());
}
Thus if I create a file yesterday at 10am and I test the file using:
ChronoLocalDate chronoLocalDate = LocalDate.now().minus(1, ChronoUnit.DAYS);
This will pass at 9am but fail at 11 am:
FileUtils.isFileNewer(file, chronoLocalDate);
I do not think this is desired behaviour. I have added notes to the javadoc to
state that the time is assumed to be now.
However I think it may be better to use LocalTime.MIDNIGHT for consistency.
Thus if you input only a date the time is assumed to be zero.
Alex
-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
Re: [dbcp][pool] Use abort instead of close for abandoned connections?
On 23/09/2020 01:29, Phil Steitz wrote: > > On 9/14/20 10:10 AM, Gary Gregory wrote: >> On Mon, Sep 14, 2020 at 1:07 PM Phil Steitz >> wrote: >>> >>> On 9/14/20 9:36 AM, Gary Gregory wrote: This feature is now in Pool master. I will prepare an RC soon if you all think we are good to go so we can then move on to DBCP. >>> I am still working on testing this in the DBCP use case. Probably best >>> to wait a little for others to review and for me to get the DBCP change >>> tested against current pool sources. I should be able to finish that >>> this weekend. > > I implemented changes in DBCP, based on recently committed changes in > pool. I made a few decisions that I would appreciate feedback on. > > 1. The JDBC abort method requires an Executor as actual parameter. I > added a FixedThreadPool executor with a max of 3 threads to > PoolableConnectionFactory for this purpose. Given that this operation > might hang sometimes, I thought it best to allow more than a single > thread. I guess it could be configurable, but 3 seemed a reasonable > choice. I copied the custom thread factory used by pool's EvictionTimer > to source daemon threads based on ccl for this. I then added a close() > method to PCF that closes the executor and modified BasicDataSource > close to call this. > > 2. I used p.getObject().getInnermostDelegate().abort(executor) in PCF > destroyObject when abandoned mode is passed in rather than just > getObject().abort as destroy invokes close (I wonder if that is a bug?) > > 3. I changed JdbcBridge#abort to remove the checkOpen() check. I was > getting exceptions in my concurrency tests where connections were being > closed but then considered abandoned. This is a legit race that abort > will generally handle fine and I don't see the need to throw. All looks reasonable to me. Mark - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
