Ok I get it :)
(I know this is not defined like this in the spec, but
should it be simpler to precise ETag on GET (block2) ? This
way server may be able to return the right block or just
said I don't have this block anymore)
That does not
really help. REST enables the server to be simple. This is
what we want to exploit, and hence put the servers on the
small devices (which are sensors and actuators, and hence
naturally the origin of data). Thus, the client should not
attempt to burden the server by asking for specific
representations.
This shows, that we need more
information than just the initial request in the shared
persistence store. We need the Exchange object as usual, and
actually extend it with the ETag information to have this
feature.
Sharing the whole Exchange object does
not make sense. (see how many complex attributes there is in
this class)
We should determine which attributes we need to share.
We currently share the request, the correlationContext. This
conversation show that we will surely need the ETag
information too.
About blockwise status, I think we should not shared it
between instance, we should keep it in memory. (We don't need to handle cross instance
blockwised transfer)
As I said, I do
not care how you implement the shared ObservationStore.
Optimize the hell out of it! :D
It simply must
store all the information necessary to run the protocol
correctly.
It is indeed missing in 1.0.x and must
be added for correct behavior. (We cannot do anything if a
server does not include an ETag, but sends out notifications
faster than all blocks can be retrieved.)
My current PR doesn't handle this
correctly but if this is not implemented in 1.0.x, I think
this should be part of another PR than the clustering one ?
I would like to limit the scope of the PR or I'm afraid we
never integrate it :/
Yes, sure. This
work simply triggered a lot of concerns that should be
answered. Yet of course the changes should be done
incrementally through nicely packaged pull requests.
My recommendation for the
implementation is to have the ObservationStore Interface,
with the local hashmap class and the shared persistence
store class as described in my other mail.
(not sure to see which mail you talked
about)
Copied: “I
agreed on the separate ObservationStore. I understood that
it will have an interface, so it can be backed by a local
implementation similar to the hashmaps we already have. In
cluster mode, it is backed by a class that connects to an
external shared persistence store, so each node can be made
aware of existing Observe relationships. I don't care if
this class creates an Exchange object every time or actually
persists the original. The Exchange object returned through
the interface must simply have all the information necessary
to process the incoming message locally. As long you do not
remove the entry from the shared persistence store, I don't
see the Exchange as completed, but it will not be in the
local hashmaps anyway. We will not need it in the
Deduplicator either, if you have a NotificationOrderer
further up...”
Whoever decided to observe something,
must let one node do the registration, but also make sure
the information is put into the shared persistence store
(maybe by the node doing the registration). Once this
decision maker decides to cancel, it must remove the
information from the shared persistence store. Then, any
node will reject the next notification. Additionally, one
node could be tasked with a proactive cancellation.
This is what I tried to implement in the
current PR.
(The tests we did in Leshan seems to show that this works)
https://github.com/eclipse/leshan/pull/113
Okay. This is
still a bit clouded for me, since I do not understand the
idea behind the NotificationListeners.
Then, the initial response as well as
all notification must go to the same handler. This should
not be different from a regular response handler.
Currently the initial response is not
accessible through the notificationListener but I can change
it.
Even if I'm not totally convinced that was a good idea ...
The spec clearly made the difference between the
registration and notification (https://tools.ietf.org/html/rfc7641#section-1.2)
The central decision maker must be
informed when the initial response does not contain the
Observe option; it then needs to fall back to polling and
task a node with it.
This reinforce that the initial response
and the notifications should not be handle in a same way.
You must be
prepared to receive a response without Observe option at any
time. The “usual” case is when you receive an error message.
The initiative is with the client to ensure it gets the
representations it wants.
Anyway, this is pretty much what it is currently implemented
in the PR. MessageObserve allow to get the initial response
and so knowing if the observation is ok or not and so fall
back to polling if necessary.
NotificationListerner is used to get notifications (We just
need to decide if the first response is a notification or
not)
You still did
not answer my question from previous mails and the PR
comments: what is the idea behind the list of
NotificationListeners? Does each one correspond to a node in
the cluster? How does this help?
Now the NotificationOrderer is still
missing. As noted in the other mail (or was it GitHub?),
this depends on the application. It could be a similar thing
to the ObservationStore that is synced across all nodes. Or
it could be done in the handler itself when writing to a
database (also in a synchronized fashion).
Ok, I think this should be part of
another PR too.
It should, but
you cannot ignore this for the architecture. You will need
something similar to the shared ObservationStore that
ensures node A does not process an old notification after
node B processed the newest. You also must ensure a
synchronized write to a database. And you need to make sure
this decision maker has a consistent view on the status of
the Observe relationship.
So far we used
a ConcurrentHashmap for performance reasons. Responses and
Notifications are in the same hashmap, and hence we can have
a race condition of two notifications climbing up the stack
in different stacks. The synchronization was only done for
notifications (the NotificationOrderer). Now that we have to
notification exchanges in a separate store and need to
synchronize it anyway, we could use this directly to order
the notifications (i.e., only accept the newest and let it
climb up the stack to the handler).
Now that the
ordering is solved (:P), why do we need a list of
NotificationListeners?
Does this help?
Yes it does, thanks for your time
Matthias ;).
I hope my answer show that the content of the current PR is
near of what you expect.
(I will not be available the next 3 weeks, so no surprise if
I don't answer during this period)
Okay, I hope it
is enjoying vacation :)
It would be
great if you could solve the riddle of the
NotificationListeners before you leave! ;)
Ciao
Matthias
