Hi Pierre,
On Sep 3, 2013, at 4:11 PM, Pierre Sutra <pierre.sutra(a)unine.ch> wrote:
Hello,
> ^ It'll be easier for you to keep up with the latest master changes in
> upstream if you work on topic branches instead of your fork's master [1]
If I understand correctly, this does not provide me with a copy on a
remote stable storage. Besides, I am not the only one to work on these
modifications.
^ You can always set up remote for the upstream repo, fetch those regularly and rebase
them. The idea is that you use your fork's master as a place to put the changes from
upstream, and then you rebase your topic branches with this. This is very well explained
in our contribution guide [1].
Also, nothing stops you from pushing branches to your fork and sharing them with others.
You colleagues can set you up as remote, pick your stuff, work on it, and you pick them,
as remote too.
> ^ Rather than pushing casting to users, it'd be better to do it
> internally, i.e. with a definition like this: <T> T
> getOrCreateInstanceOf(Class<T> clazz,…
It was not meant to be the final API, and I will change this. (despite I
am not found of Java generics)
I guess you're not fond of of Java Generics, but they're there to help, and used
them sensively they can avoid horrible casts.
>
> Also, getOrCreateInstanceOf name feels a bit verbose to me. Why not
just call it 'create'? Alternatively, we already have code to create
atomic maps in AtomicMapLookup. We might wanna merge both into a single
factory/lookup class. Not a biggie though...
It is named getOrCreateInstanceOf because you may create a new object
but also, in case the object exists already in the system, receive a
consistent copy of it.
^ That's an implementation detail IMO. Not sure that should feed into the name of the
method. IOW, imagine if Map.get was called Map.getOrMaybeNull? :)
The two approaches of building maps are radically different, and they
will give radically different results. Nevertheless, I agree that
interfacing the two constructs with a single API would be of interest.
This is something I will eventually investigate.
>
>> set.add("smthing"); // some call examples
>> System.out.println(set.toString())
>> set.addAll(set);
>> factory.disposeInstanceOf(HashSet.class, "set", true); // to store the
>> object in a persistent way
>>
>> The pattern I followed to implement this facility is the state machine
>> replication approach. More precisely, the factory is built on top of the
>> transactional facility of Infinispan.
> ^ Why do you need transactions? Can you just call put?
The approach I am following to implement linearizable (aka. atomic)
objects [1] is a variation of state machine replication [2]. At core,
state machine replication requires total order broadcast to propagate
the modifications - a property given by a push in transactional mode in
Infinispan.
Ok
>> When the factory creates an
>> object, it stores a local copy of the object and registers a proxy as a
>> cache listener.
> Hmmm, javassist based proxying. This needs some thought and
consideration. I'm not a huge expert on this topic, but we have
Hibernate crew listening here which might be able to provide feedback on
how to best bridge over, since it's something they've been doing since
day one.
This approach seems to do a fairly good job and is both fast and
invisible to the user.
> If you're just calling a single cache operation, don't see the reason
for a transaction. You still get notifications from other servers if
something fails. The only difference is lack of of rollback if something
goes wrong in one of the servers and it works fine in others. Not using
transactions makes things work faster.
I could use a non-transactional cache, provided I have access to an
atomic broadcast primitive. For the moment, I did not find a
non-intrusive solution to this issue.
>
>> When the call is
>> de-serialized it is applied to the local copy, and in case the calling
>> process was local, the result of the call is returned (this mechanism is
>> implemented via a future object).
> ^ Why a future?
The result of the call is received via a call-back triggered by
modifications on the object's key.
>> Notice that this implies that all the
>> arguments of the methods of the object are serializable, as well as the
>> object itself.
> Serializable, or somehow marshallable… [2]
I will add this feature to the code (e.g., by giving a marshaller object
to the factory).
>
>> The current implementation supports the retrieval of
>> persisted objects, an optimization mechanism for read operations,
> ^ Can you explain this read optimization in more detail? Why do you
need it?
>
> I'm also seeing a lot of marshalling in your code, which I don't
understand why you need it at all
The idea is to optimistically execute each operation locally
(marshalling here is employed to obtain a deep copy of the object). If
the operation does not modify the object, then it is safe to return the
result of its tentative execution to the caller. Notice here that,
strictly speaking, activating this feature makes the object not atomic
but sequentially consistent [3].
^ Hmmmm, are you aware that we have defensive copying in place now which allows you to
work on the object without affecting the contents of the cache? Maybe enabling that would
allows you do what you need without you having to fidle with further marshalling?
Thanks a lot for working on this!
As I said in my previous email, we'd strongly recommend that you send a pull request
with the code so that we can more easily comment on things.
Cheers,
[1]
https://docs.jboss.org/author/display/ISPN/Contributing+to+Infinispan
[2]
https://docs.jboss.org/author/display/ISPN53/Marshalling#Marshalling-Stor...
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org