----- Original Message -----
From: "Noel J. Bergman" <[EMAIL PROTECTED]>
> > I thought that the Watchdog as an abstraction lacked
> > a few important things. These were the things that I
> > thought were worth having :
> > Support m watchdog threads watching n handlers.
> > Publish 'resettable, time-guarded' as a pluggable service/block.
>
> You are confusing ABSTRACTION with IMPLEMENTATION. The ABSTRACTION of the
> Watchdog interface is that client code has an opaque object with which to
> interact using a small set of operations, e.g., start, stop, reset.
This is exactly what the Abstraction I sent earlier has, except there is an
associated handler key. Instead of start, stop, reset methods are add,
remove, reset.
We seem to be arguing if the handler key should be there or not in the
Abstraction.
> The Watchdog is a just portable, lightweight interface. If someone wanted
> to create a Block that provided Watchdog instances, they could.
Would it be possible to provide this as part of the proposal/patch. James
has been following a Component oriented approach elsewhere it would be nice
to follow it for the watchdog too.
>
> > Handlers should not do this: foo = new SomeResetableTimeGuard();
>
> You mean like in the current (CVS) code? Where it has:
>
> final PeriodicTimeTrigger trigger = new PeriodicTimeTrigger(
> timeout, -1 );
I was thinking something like
in config object (i.e. once per n handlers)
ResettableTimeGuardedCommandMap timerEventMgr =
componentManager.lookup(ResettableTimeGuardedCommandMap.ROLE);
In handler code.
this.handlerTimeoutEventID = timerEventMgr.add(timeout,this);
There is no PeriodicTimeTrigger object or any other heavyweight object
created.
>
> Thanks for bringing this up. Good news. Watchdog creation is no longer
in
> the Handler. Handlers are given a Watchdog. They are ignorant of the
> implementation of that interface.
Is watchdog part of the context provided ?
>
> > ResettableTimeGuardedCommandMap was a suggestion
> > to facilitate arriving at a good abstraction.
>
> It is a completely different approach to handler implementation that
> requires rewriting Handlers to use a command map. I had submitted code
for
> that over the summer, but I would not propose rewriting handlers for 2.1.
> IF we rewrite the handlers for v3 to use a command map, changing how
> handlers do timeout will be the least of our issues.
Attaching what would be the diff for SMTP Handler. It is pretty similar to
the current code and not very far from watchdog either. It certainly is
really far from rewrite :-)
BTW. I didn't like the name ResettableTimeGuardedCommandMap but I wanted the
name to derive from proposal subject and not have either watchdog or
scheduler word in the name.
I would prefer we arrive at one abstraction and not revisit this for a few
years. Let us try to settle on something really good now and not revisit for
v3 etc.
>
> > Logging can be configured to ...
>
> Logging support is provided to James by Avalon, and as you said, "Logging
> facilities seem pretty flexible and good to me as it is."
Good to get agreement on this. Patch really has nothing to do with logging
improvements.
>
> > Latter looks fine to me. I agree with your comments on information
hiding.
> > Is the configure object something like the one attached?
> > with POP3Handler having something like
> > void setConfiguration(POP3HandlerConfig config)
>
> public interface POP3HandlerConfigurationData {
> String getHelloName();
> int getResetLength();
> MailServer getMailServer();
> UsersRepository getUsersRepository();
> }
>
> void setConfigurationData(POP3HandlerConfigurationData theData);
Looks good.
>
> > Do you agree that the config optimization could be
> > done in a more localized manner?
>
> By localized, you mean you just want the change for configuration, and no
> other code change? The changes I am aware of are:
Config changes and replacing scheduler with a lighter weight
resettable-timeguarded-operations Abstraction.
>
> - handler configuration
> - this requires the server to prepare the service-wide
> configuration.
Part of the confiuration change.
> - this permits object pooling of handlers and related objects.
Handler object would be very lighweight using POP3HandlerConfigurationData.
It would prob. only contain a socket, Input Output Stream object and not
much else. Socket related objects cannot be pooled.
> - Watchdog interface
This is part of resettable-timeguarded-operations Abstraction change. I like
your original proposal subject name better.
> - Unlike the Scheduler interface, this interface hides
> implementation details of timeout from the client
> code, and supports arbitrary timeout mechanisms
> transparently.
> - Support for per-service (POP3, SMTP, ...) and global
> connection limits.
Will there be one connection limits for pop3 and pop3 secure ?
Connection limits are already imposed by current code.
>
> > Why pull and duplicate code ?
>
> It doesn't. Thread management has always been part of James, provided by
an
> Avalon thread manager. That thread manager still does its job. What DOES
> open is the OPTION of having additional <thread-group> elements. And
James
> is now able to enforce connection limits per service.
Alreay there in current code.
>
> AbstractJamesService does not "pull in thread management" or even use it.
> What it does do is provide better control over thread pooling. It simply
> finds out from the service configuration if a specific pool is to be used
> for the service, gets either the specified pool or the default pool from
the
> thread manager service, and passes that onto the ConnectionManager.
Threads
> are still managed by the thread manager, and connections are still managed
> (and dispatched on threads) by the connection manager.
I realize it is pretty similar. What I don't know is this if we are adding
value to Avalon's service, why don't we push this into Avalon. There are lot
of common Avalon/James commiters who could do this.
> There is nothing wrong with Avalon's base service. But it does not model
> the behavior common to Jame's services.
There are upto 5 protocol handlers in James, Most things common for James
would be useful would be useful for other servers too. So again, why pull in
code into james and not push changes as need be into Avalon.
If we have found a better pattern of doing listener-dispatch, we might as
well push it in a more general location.
To me, it would be way cooler to say: would be nice if Avalon did xyz and
get them to do add it via a patch.
>
> AbstractJamesService models behavior for James services. It provides a
> shared implementation for derived classes, handling such common model
> elements as bindaddress, port, enabled, useTLS, helloName, timeout, and
> connection limits; behavior such as initialize and dispose; and exposes
the
> HandlerFactory interface.
All of these can be handled by you ConfigData object. Right ?
Anyway I don't think it is a big deal. I have been a bit apprehensive about
the amont of redisgn for fixing things. Even for this fix, why don't we
simply get to the bottom on config passing that we have agreement on and
come up with a good 'resettable-timeguarded-operations' abstraction that we
are pretty close and leave out Refactoring unless you see a huge gain.
Harmeet
--
To unsubscribe, e-mail: <mailto:james-dev-unsubscribe@;jakarta.apache.org>
For additional commands, e-mail: <mailto:james-dev-help@;jakarta.apache.org>
