Thats a good summary, couldnt write it better myself.
As for the questions:
> Synchronization issues in PushDataManager: In the early commits this was
> inconsistent, are you happy with it now?
No, I'm not happy with it yet. It has a fatal bug in there, and I'll
need to debug it. Sometimes a closed tab has make it to the
awaitingNotifications, and it leaks memory.
> Hidden input for the request ID (in PageMaker): is this valid HTML? IIRC it's
> not inside a form?
Firefox says it's valid, so I think it is
> Is separating data from notifications more efficient, even though it involves
> more round-trips?
It is needed, because data can be any size, but cookies has a max. It
does involves more round trip, but it's just going to localhost, and
thats fast.
> Re "+" in base64, you could just use a different encoding - the freenet
> nonstandard base64 for example. You can easily change the base64 code used in
> the java code to deal with this? OTOH you could avoid it completely with some
> changes to the wire format...
The "+" is the only problematic character, that is in the BASE64
coding and HTML escaping too. As for now, it hasn't gave me too much
headache, but if it will in the future, I'll consider changing.
> In the cleaner thread:
> + for (Entry<String, Boolean> entry : new
> HashMap<String, Boolean>(isKeepaliveReceived).entrySet()) {
> ...
> +
> isKeepaliveReceived.put(entry.getKey(), false);
>
> Is this safe? You don't get ConcurrentModificationException's?
It isn't modified concurrently, because a new HashMap is created from
the isKeepaliveReceived map, and it's entrySet is being iterated.
>
> Please use Ticker rather than java timers, because of thread pooling and
> thread priority issues.
K, will rewrite it
> Would it make sense to randomise the failover interval slightly, so that one
> fails over and then the other two don't need to?
The failover is randomised, because it is based on the loading of the
page. It is different for every tab.
> You are assuming we will almost never reach MAX_MESSAGES, and in that case
> the output will just be a little out of date, this is probably reasonable.
MAX_MESSAGES is the maximum difference between the leader's message
count and the followers'. It shouldn't be reached, because it would
need that the leader has receive events in a 100/10=10ms interval, and
thats not possible, an xmlhttpconnection needs more time. If magically
the MAX_MESSAGES is hit, then some notifications will be lost.
> PushDataManager.awaitingNotifications only contains the "leaders", correct?
> But all events are broadcast to all leaders? (Javadocs for the variables
> would be nice)
All events are broadcast to all leaders, yes. There is one leader for
every browser, and because we dont know which request belongs to which
browser, we need to broadcast. And because only leaders asks for
notifications, we need only to dispatch them to leaders. Thats what
the serverside failover does. It copies all the notifications from the
old leader's list to the new one.
I've only did the coding part yet. When it seems finalized, I'll write
comments, javadocs and some documents about how it works and how ppl
can code against it.
For the build process to be integrated, I'll need the 2 jars to be
publicly available somewhere. If it is, then everybody could build
even after a distclean.
sashee
_______________________________________________
Devl mailing list
[email protected]
http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
