Congrats Ovidiu. Some early comments:
I want to see some work done on the persistence layer, in particular the equivalent of these tasks for JBossMQ: http://jira.jboss.com/jira/browse/JBMQ-12 http://jira.jboss.com/jira/browse/JBMQ-11 These will be the most important aspects for performance and scalability and they can't just be bolted on later without proper consideration of how they should be plugged in. I don't see any notion of MessageCache for scalability. I also don't see any notion of ordering of messages (another important feature). Finally, I don't like this style of code: | public boolean deliver() | { | synchronized(channelLock) | { | // try to flush the message store | for(Iterator i = unacked.iterator(); i.hasNext(); ) | { | Routable r = (Routable)i.next(); | try | { | if (receiver.handle(r)) | { | i.remove(); | } | } | catch(Throwable t) | { | // most likely the receiver is broken, don't insist | break; | } | } | return unacked.isEmpty(); | } | } | LOCKING 1) It is usually not a good idea to "call out" from inside a synchronized block. You loose all hope of trying to control the locking strategy, in particular avoiding deadlocks. 2) It is even worse in this case since Receiver is something you let other people plugin. 3) If you look at my earlier prototype, this was deliberatley done in two stages http://cvs.sourceforge.net/viewcvs.py/jboss/jboss-jms/src/main/org/jboss/messaging/channel/plugins/handler/Attic/AbstractChannelHandler.java?annotate=1.1 lock(); try { decideWhatToDoForOneMessageWhileWeHaveGuaranteedState(); } finally { unlock(); } doWhatWasDecidedInsideTheLock(); ERRORS AND LOGGING 1) It is a good that you are catching Throwable but you should never just eat error messages without some sort of logging. 2) I would expect to see some TRACE logging that will allow developers to see what is going on when resolving problems. e.g. Look at what JBossMQ does (where every major event is logged at tracel level): http://cvs.sourceforge.net/viewcvs.py/jboss/jbossmq/src/main/org/jboss/mq/server/BasicQueue.java?annotate=1.42.2.1 View the original post : http://www.jboss.org/index.html?module=bb&op=viewtopic&p=3862814#3862814 Reply to the post : http://www.jboss.org/index.html?module=bb&op=posting&mode=reply&p=3862814 ------------------------------------------------------- This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting Tool for open source databases. Create drag-&-drop reports. Save time by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc. Download a FREE copy at http://www.intelliview.com/go/osdn_nl _______________________________________________ JBoss-Development mailing list JBoss-Development@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jboss-development
