On 27/07/2009, at 9:06 AM, Emmanuel Lecharny wrote:
<snip>
Matthew Phillips wrote:
On 26/07/2009, at 10:19 PM, Emmanuel Lecharny wrote:
As an alternative, I could have the filter keep its own stats in an
AtomicLong.
This is what we used in the IoServiceStatistic class. It guarantees
you that the value is not complete garbage in a concurrent env.
I'm sure I'm not telling you anything new here, but it does seem
that any reasonable attempt at handling misbehaved clients would
need such stats, and handling DOS's (accidental or deliberate) is
something nearly every MINA-based app will need to do if it gets
deployed in a production environment.
Well, I would say that any decent production system would use
another system to protect the server against DOS (at least,
deliberate ones). Now, you are right, it's probably a good idea to
have a good set of *valid* counters at some point. The only problem
is to have them implemented correctly ;)
PS: This is an area where MINA can most certainly benefit from some
large improvement...
Are there any plans to address this before MINA 2.0 is finalised? I'm
on a tight schedule and need to either fix this in MINA, or do it some
other way in the next two weeks or so. If a fix might make its way
into MINA, I might be able to justify doing it "properly".
Assuming I go the "proper" route, it seems like IoSession could use an
equivalent IoSessionStatistics class like IoService has.
Thinking about it more, I don't like the idea of having a stats API
built into IoSession, but it only being enabled if you add the right
filter in the right place, and silently failing if you don't.
One way to fix this would be to have an "IoSession.setStatistics
(IoSessionStatistics newStats)" method which would enable stats
counting (and optionally allow stats from a previous session to be
preserved in the case where a session is re-established, which is
something I could also use). The IoSession would update the stats
itself, but only if enabled: I don't see this adding any measurable
overhead, just a test for statistics == null and a jump in the case
where no stats are needed.
If IoSessionStatistics were an interface/abstract base class, then
this could even be re-implemented to do custom actions on stats changes.
Some API sample code:
public abstract class AbstractIoSession
{
private IoSessionStatistics statistics;
...
public void setStatistics (IoSessionStatistics newStatistics)
{
statistics = newStatistics;
}
/* Actually I'd prefer just "statistics ()", but I think MINA uses
JavaBeans property patterns everywhere else. */
public IoSessionStatistics getStatistics ()
{
return statistics;
}
// Shortcut for the majority who don't care which stats instance is
used
public void setStatisticsEnabled (boolean enabled)
{
if (enabled)
{
if (statistics == null)
statistics = new DefaultIoSessionStatistics ();
} else
{
statistics = null;
}
}
public boolean isStatisticsEnabled ()
{
return statistics != null;
}
// current stats API either removed or implemented as
public final double getReadMessagesThroughput ()
{
if (statistics == null)
throw new IllegalStateException ("Statistics not enabled");
else
return statistics.getReadMessagesThroughput ();
}
...
}
