On Thu, 2006-03-09 at 10:07, David M Johnson wrote: > I'm definitely -1 on part item 2 of your proposal. > > I do not understand what you are proposing to do and I'd like to see > specifically how each action and method in the back-end will change. > There are many multi-step queries, updates, inserts, etc. in the code > and application logic is expressed in both Struts actions and in the > back-end. I don't think you are adequately explaining how you will > keep transaction demarcation in place. And I don't understand how you > can move begin() and commit() the back-end without moving lots of > code from the Struts actions into the manager classes.
Doing an exhaustive explanation of every piece of code that would move is pretty tough. anything that truly requires a transaction would be moved into a logical method in a manager class. > > To do this right you'll have to do some trail and error > experimentation and in the end I believe you'll have to make > extensive changes. I'd like to have community review of those changes > before they goes in trunk. For that reason, I strongly suggest doing > item 2 in a separate branch. yes, the changes will touch a number of classes and in the case of some of our larger transactions we may need to push some logic from the action classes into the manager classes. i find that in *most* cases an action class is really only working on a single object at a time, so i don't really think we do very long and complicated transactions. i am fine with doing this work in a branch. i think this discussion is losing sight of its original purpose though, so i'd like to step back for a second and just talk about why i think the backend needs some reforming. 1. the first and foremost issue is performance. i have spent a considerable amount of time over the recent months evaluating where i believe Roller is having performance bottlenecks and trying to sort them out as best i can. after looking more closely at Hibernate as it is used by Roller i have seen some very troubling performance issues. for example, we are generating more traffic talking to the db than we are serving requests and the Hibernate L2 cache is of essentially no help to us right now. this is a huge problem for large sites and it has to be fixed. 2. while doing my debugging on the performance issues i noticed that the current Roller transaction demarcation code really isn't implemented completely. calls to the begin() methods are used somewhat sporatically, and most important, calls to rollback() are non-existent. i don't know how we can claim true transaction support if we aren't doing rollbacks. we also are not using Hibernates Transaction wrappers either, which is highly recommended in the Hibernate docs. this leads me to believe that we need a bit of an overhaul on the general backend code structure. 3. finally, because of the issues that i saw from item #2 i wanted to suggest what i believe is far more simplified approach than what we have now. that approach is quite simply to push all the transaction demarcation code back behind the XXXManager implementations and let the various persistence classes take control over that logic. this pulls a reasonable amount of code outside of the web layer and allows our servlets and struts actions to do persistence operations in a more abstracted fashion. i.e. rather than creating 6 objects and saving them in a struts action as part of new weblog creation, instead we have a method called createWeblog() which takes in all the required data and handles the details of how to persist that data. i believe this approach will work fine for Roller and since i am already planning to dig into the backend code to resolve issues #1 and #2 i thought i would suggest making some simplifications (#3) at the same time. at the end of the day this may be a case where either method will work and it's just a matter of choosing one. we can either upgrade/fix our current transaction demarcation code or migrate to the approach i proposed which pushes the transaction logic behind the manager classes. there may not be a "correct" approach. -- Allen > > - Dave > > > On Mar 8, 2006, at 6:40 PM, Allen Gilliland wrote: > > > just a reminder, silence == consent. i am planning to move forward > > with development on this. > > > > -- Allen > > > > > > On Mon, 2006-03-06 at 13:25, Allen Gilliland wrote: > >> Okay, here is a more flushed out proposal of what I think should > >> be done ... > >> > >> http://rollerweblogger.org/wiki/Wiki.jsp? > >> page=Proposal_BackendRefactorings > >> > >> Note that the driving force behind these changes is to 1) prevent > >> direct access to the PersistenceStrategy from outside the business > >> layer and 2) limit persistence logic (like transaction details) to > >> the business layer. > >> > >> I think those are very appropriate goals to have for the > >> architecture of our code. > >> > >> -- Allen > >> > >> > >> On Sun, 2006-03-05 at 10:10, Anil Gangolli wrote: > >>> Anil Gangolli wrote: > >>>> > >>>> Actually, you want those two queries to be in one transaction, not > >>>> two; otherwise they can see inconsistent state if there is another > >>>> transaction committed between the two. > >>>> > >>> Red herring on my part. Not really a new issue for us though > >>> since we > >>> are typically using default READ_COMMITTED isolation anyway. > >>> > >>> --a. > >>> > >> >
