Bitronix 3.0 prep

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Bitronix 3.0 prep

Brett Wooldridge-2
Ludovic,

I have merged my github fork into btm-2.2 on codehaus.  Feel free to merge it into master and/or create a 3.0 branch.  When merging into master, the only thing you probably want to exclude is the btm-nio-journal module.  All other modules are the same as the current master.

If you want me to perform the merge, I'm happy to, just let me know.

Brett

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Ludovic Orban-2
I've merged the 2.2 and 2.1 branches together onto the master branch then I edited the pom version to 3.0.0-SNAPSHOT.

On top of that, I've done some cleaning up:

 - dropped the ZIP archive subproject
 - dropped the nio journal (it still lives in the 2.2 branch, it can be revived at any time)
 - upgraded the maven plugins
 - cleaned up the poms and moved cglib and javassist to the "provided" scope
 - dropped all references to the jdbc-3.0 jar and erased it
 - fixed the java.sql.Wrapper implementations
 - more misc stuff & small bug fixes

We're coming close to a first release candidate. If anyone has something to add or want to test a new feature or bug fix, act now as my guesstimate is that BTM 3.0.0 should be released in the very first weeks of 2013. I'm going to start seriously testing all that any time soon.

Thanks to everyone who helped one way or another, or who simply are happy BTM users!

On Fri, Dec 7, 2012 at 4:37 AM, Brett Wooldridge <[hidden email]> wrote:
Ludovic,

I have merged my github fork into btm-2.2 on codehaus.  Feel free to merge it into master and/or create a 3.0 branch.  When merging into master, the only thing you probably want to exclude is the btm-nio-journal module.  All other modules are the same as the current master.

If you want me to perform the merge, I'm happy to, just let me know.

Brett


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

snicoll
Nice! Looking forward 3.0

Cheers,
S.


On Fri, Dec 7, 2012 at 1:22 PM, Ludovic Orban <[hidden email]> wrote:
I've merged the 2.2 and 2.1 branches together onto the master branch then I edited the pom version to 3.0.0-SNAPSHOT.

On top of that, I've done some cleaning up:

 - dropped the ZIP archive subproject
 - dropped the nio journal (it still lives in the 2.2 branch, it can be revived at any time)
 - upgraded the maven plugins
 - cleaned up the poms and moved cglib and javassist to the "provided" scope
 - dropped all references to the jdbc-3.0 jar and erased it
 - fixed the java.sql.Wrapper implementations
 - more misc stuff & small bug fixes

We're coming close to a first release candidate. If anyone has something to add or want to test a new feature or bug fix, act now as my guesstimate is that BTM 3.0.0 should be released in the very first weeks of 2013. I'm going to start seriously testing all that any time soon.

Thanks to everyone who helped one way or another, or who simply are happy BTM users!

On Fri, Dec 7, 2012 at 4:37 AM, Brett Wooldridge <[hidden email]> wrote:
Ludovic,

I have merged my github fork into btm-2.2 on codehaus.  Feel free to merge it into master and/or create a 3.0 branch.  When merging into master, the only thing you probably want to exclude is the btm-nio-journal module.  All other modules are the same as the current master.

If you want me to perform the merge, I'm happy to, just let me know.

Brett



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Brett Wooldridge-2
In reply to this post by Ludovic Orban-2
Bummer about the jdbc-3.0.jar.  It is that jar that allows Bitronix to be compiled under any development environment (JVM).  For example, Java 5 is not available on Mac OS X any longer.  If that jar is not present, Bitronix will not compile under Java 6 or Java 7 (on any platform).  I personally don't consider that a good thing, and know of no other open source projects that I build that require a specific version of the JVM.  It is not a shipped artifact and in no way affects deployment, so my preference would be that we include it in the compile-time dependencies.

Just my opinion.

Brett



On Fri, Dec 7, 2012 at 9:22 PM, Ludovic Orban <[hidden email]> wrote:
I've merged the 2.2 and 2.1 branches together onto the master branch then I edited the pom version to 3.0.0-SNAPSHOT.

On top of that, I've done some cleaning up:

 - dropped the ZIP archive subproject
 - dropped the nio journal (it still lives in the 2.2 branch, it can be revived at any time)
 - upgraded the maven plugins
 - cleaned up the poms and moved cglib and javassist to the "provided" scope
 - dropped all references to the jdbc-3.0 jar and erased it
 - fixed the java.sql.Wrapper implementations
 - more misc stuff & small bug fixes

We're coming close to a first release candidate. If anyone has something to add or want to test a new feature or bug fix, act now as my guesstimate is that BTM 3.0.0 should be released in the very first weeks of 2013. I'm going to start seriously testing all that any time soon.

Thanks to everyone who helped one way or another, or who simply are happy BTM users!


On Fri, Dec 7, 2012 at 4:37 AM, Brett Wooldridge <[hidden email]> wrote:
Ludovic,

I have merged my github fork into btm-2.2 on codehaus.  Feel free to merge it into master and/or create a 3.0 branch.  When merging into master, the only thing you probably want to exclude is the btm-nio-journal module.  All other modules are the same as the current master.

If you want me to perform the merge, I'm happy to, just let me know.

Brett



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Ludovic Orban-2
This is something I overlooked since I am using JDK 1.6 by default, everything just worked. I tried building both with JDK 1.5 and 1.7 and indeed, the compilation failed.

I've re-inserted the jdbc-3.jar file and modified the pom, after a shot dose of cleanup. That made me discover a concurrency issue in the task scheduler when running on JDK 1.5 by the way, which I also fixed.

Thanks for the reminder!

On Fri, Dec 7, 2012 at 2:27 PM, Brett Wooldridge <[hidden email]> wrote:
Bummer about the jdbc-3.0.jar.  It is that jar that allows Bitronix to be compiled under any development environment (JVM).  For example, Java 5 is not available on Mac OS X any longer.  If that jar is not present, Bitronix will not compile under Java 6 or Java 7 (on any platform).  I personally don't consider that a good thing, and know of no other open source projects that I build that require a specific version of the JVM.  It is not a shipped artifact and in no way affects deployment, so my preference would be that we include it in the compile-time dependencies.

Just my opinion.

Brett



On Fri, Dec 7, 2012 at 9:22 PM, Ludovic Orban <[hidden email]> wrote:
I've merged the 2.2 and 2.1 branches together onto the master branch then I edited the pom version to 3.0.0-SNAPSHOT.

On top of that, I've done some cleaning up:

 - dropped the ZIP archive subproject
 - dropped the nio journal (it still lives in the 2.2 branch, it can be revived at any time)
 - upgraded the maven plugins
 - cleaned up the poms and moved cglib and javassist to the "provided" scope
 - dropped all references to the jdbc-3.0 jar and erased it
 - fixed the java.sql.Wrapper implementations
 - more misc stuff & small bug fixes

We're coming close to a first release candidate. If anyone has something to add or want to test a new feature or bug fix, act now as my guesstimate is that BTM 3.0.0 should be released in the very first weeks of 2013. I'm going to start seriously testing all that any time soon.

Thanks to everyone who helped one way or another, or who simply are happy BTM users!


On Fri, Dec 7, 2012 at 4:37 AM, Brett Wooldridge <[hidden email]> wrote:
Ludovic,

I have merged my github fork into btm-2.2 on codehaus.  Feel free to merge it into master and/or create a 3.0 branch.  When merging into master, the only thing you probably want to exclude is the btm-nio-journal module.  All other modules are the same as the current master.

If you want me to perform the merge, I'm happy to, just let me know.

Brett




Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Jaco de Groot
In reply to this post by Ludovic Orban-2
I'd like to see me patches mailed earlier and added to BTM-83 being
added to the new release when possible.

Regards, Jaco

Op 07-12-12 13:22, Ludovic Orban schreef:

> I've merged the 2.2 and 2.1 branches together onto the master branch
> then I edited the pom version to 3.0.0-SNAPSHOT.
>
> On top of that, I've done some cleaning up:
>
>   - dropped the ZIP archive subproject
>   - dropped the nio journal (it still lives in the 2.2 branch, it can be
> revived at any time)
>   - upgraded the maven plugins
>   - cleaned up the poms and moved cglib and javassist to the "provided"
> scope
>   - dropped all references to the jdbc-3.0 jar and erased it
>   - fixed the java.sql.Wrapper implementations
>   - more misc stuff & small bug fixes
>
> We're coming close to a first release candidate. If anyone has something
> to add or want to test a new feature or bug fix, act now as my
> guesstimate is that BTM 3.0.0 should be released in the very first weeks
> of 2013. I'm going to start seriously testing all that any time soon.
>
> Thanks to everyone who helped one way or another, or who simply are
> happy BTM users!
>
> On Fri, Dec 7, 2012 at 4:37 AM, Brett Wooldridge
> <[hidden email] <mailto:[hidden email]>> wrote:
>
>     Ludovic,
>
>     I have merged my github fork into btm-2.2 on codehaus.  Feel free to
>     merge it into master and/or create a 3.0 branch.  When merging into
>     master, the only thing you probably want to exclude is the
>     btm-nio-journal module.  All other modules are the same as the
>     current master.
>
>     If you want me to perform the merge, I'm happy to, just let me know.
>
>     Brett
>
>

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Brett Wooldridge-2
Ludovic,

I wrote up a brief summary of the changes in 3.0, you can find it here:


Regards,
Brett

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Ludovic Orban-2
Hi Brett,

Thanks for that helpful summary of your changes, they were unvaluable understanding your logic. We should get that into the official BTM wiki but the new confluence is a horrible pain to work with IMHO and I'm looking for an alternative or some trick to make it work again with a wiki syntax instead of xhtml.

I have a few more questions and remarks though. In no particular order:

 - You should always try to put as many fields as final as possible, otherwise we may trigger awful concurrency issues that are very hard to track down. I marked all the ones I could find and reviewed that the other fields are either marked volatile or always accessed from within a lock. That's a common habit we should all have.

 - I'm not always completely sure of what I'm doing with git branching and merging. I may have messed things up and that may happen again in the future until I get proficient with this tool. In the meantime, I'd be happy if you could keep an eye on my pushes to make sure I didn't do anything stupid.

 - In TransactionLogAppender.trackOutstanding() you used to lock around the 'outstanding' set before updating it. That looks clunky to me and doesn't prevent all race conditions, that just keeps the data properly visible across threads. This led me to replace the set with a Collections.synchronizedSet() to encapsulate the required data sychronization as the collected data is never going to be accessed by more than one thread at a time so there's no need for thread-safety here.

 - When DiskJournal.swapJournalFiles() would overflow, it should not just log an error but throw an exception instead. This piece of code should never be executed in theory, but in case an unknown DiskJournal bug triggers it I believe it is much more desirable to try to preserve what's left of the journal's integrity and stop all journaling than to leave the TM running with a corrupted journal.

 - When the DiskJournal is instructed to force, it should also ensure that the position is written to disk as well. I can't say if the current code does that or not so I'd be happy if you could have a look at this point. Please remember that everything "after" the header's position is considered garbage during recovery so even if some records were properly logged and forced, if the position was not updated the journal just ensured that it safely wrote... garbage! This certainly is worth being documented.

That's all I have for now. Overall, your changes are quite contained and don't look like they'd destabilize the whole product. In short, I like them even if I still need to seriously test them and continue reviewing them and making some minor changes here and there.

Thanks,
Ludovic



On Sun, Dec 9, 2012 at 7:10 AM, Brett Wooldridge <[hidden email]> wrote:
Ludovic,

I wrote up a brief summary of the changes in 3.0, you can find it here:


Regards,
Brett


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Bitronix 3.0 prep

Brett Wooldridge-2
Hi Ludovic,

When DiskJournal.swapJournalFiles() would overflow, it should not just log an error but throw an exception instead. 

I agree.

> When the DiskJournal is instructed to force, it should also ensure that the position is written to disk as well. I can't
> say if the current code does that or not so I'd be happy if you could have a look at this point.

The current code does do so.  I'll explain.

Before any thread is allowed into the TransactionLogAppender it passes though the DiskJournal's swapForceLock, acquiring the readLock.  Multiple threads are allowed to enter this lock.  When it calculates it's write position [setPositionAndAdvance()] it increments the atomic outstandingWrites.  As it exits the TransactionLogAppender.writeLog() it decrements the atomic outstandingWrites.  Whenever outstandingWrites reaches zero there are no more concurrent writes occurring, and the position header is updated on disk.

Now, on the DiskJournal.force() side, the force() must acquire the swapForceLock's writeLock().  If force() is called while there are outstanding threads in the appender, it will block because those threads hold the swapForceLock readLock.  However, once force() acquires the writeLock, it is guaranteed that the last thread has exited the appender, and also guarantees that any new append threads are blocked from entering the appender (because they need the readLock but the force() has the writeLock).  The fact that the force() has acquired the writeLock itself is a guarantee that the last appender thread has exited TransactionLogAppender.writeLog(), the outstandingWrites reached zero, and the position header was updated.  Thus when DiskJournal.force() executes TransactionLogAppender.force() while holding the writeLock, it is guaranteed that the position header is both updated and correct.

In TransactionLogAppender.trackOutstanding() you used to lock around the 'outstanding' set before updating it. 
> That looks clunky to me and doesn't prevent all race conditions, that just keeps the data properly visible across
> threads.

Not quite correct.  The synchronization was there to guarantee not only the safety of the collection (which it does), but that these two operations were atomic:

// in case Status.STATUS_COMMITTING:
Set<String> outstanding = danglingRecords.putIfAbsent(gtrid, ...);
if (outstanding != null) {
   synchronized (outstanding) {
      outstanding.addAll(uniqueNames);
   }
}

// in case Status.STATUS_ROLLEDBACK etc.
synchronized (outstanding) {
   outstanding.removeAll(uniqueNames);
   if (outstanding.isEmpty()) {
       danglingRecords.remove(gtrid);
   }
}

It is the later two operations in particular, the outstanding.removeAll() and the subsequent outstanding.isEmpty() that needed to be atomic /with respect to/ the outstanding.addAll() call.  By merely making the collection synchronized, this guarantee is lost.

Having said that, upon reviewing that code, there is still a potential race between danglingRecords.putIfAbsent(gtrid, ...) and danglingRecords.remove(gtrid).  I have removed the too-clever use of ConcurrentHashMap and putIfAbsent() and replaced with a simpler synchronization on danglingRecords itself.  Because the TreeSet is now accessed only within the context of this synchronization, it does not itself need to be synchronized (per JLS 17.4.5 Happens-before ordering).

Regards,
Brett

Loading...