I have found this to be a very interesting conversation. My own
bias is,
we are talking about 4.0. Let's focus on releasing 3.0 ;-)
Don’t get me started :-)
I completely understand Peter's concerns about reasoning about
visibility
for concurrency.
I understand from Greg that the visibility comes from the network
barrier.
How does this play out when changes are published into either a service
or
a client cache for the attributes used to publish or discover a proxy?
This would seem to be the place where there is an opportunity for
concurrency issues.
Looking at
http://river.apache.org/doc/api/net/jini/core/lookup/ServiceItem.html,
I
see that the ServiceItem is three fields:
Entry<http://river.apache.org/doc/api/net/jini/core/entry/Entry.html
[]*attributeSets
<
http://river.apache.org/doc/api/net/jini/core/lookup/ServiceItem.html#attributeSets
*
Attribute sets.
Object
<
http://java.sun.com/j2se/1.4.2/docs/api/java/lang/Object.html?is-external=true
*service
<
http://river.apache.org/doc/api/net/jini/core/lookup/ServiceItem.html#service
*
A service object.
ServiceID
<http://river.apache.org/doc/api/net/jini/core/lookup/ServiceID.html
*serviceID
<
http://river.apache.org/doc/api/net/jini/core/lookup/ServiceItem.html#serviceID
*
A service ID, or null if registering for the first time.
It seems reasonable if the attributeSets field's value is replaced.
But
there would be a visibility concern about this.
It's been a while since I wrote service cache logic. Is the visibility
guarantee here provided by an interface providing notice of
attributeSet
changes?
Interesting question. Let’s clarify it by asking “What exactly are we
doing with the ServiceItem instance”. Also, note that ServiceItem is
not
itself an Entry, it just holds an array of Entry objects.
Anyway, you mention service cache logic, so let’s look at
ServiceDiscoveryManger and LookupCache:
In the simple case, I call 'lookup(ServiceTemplate tmpl, int minMatches,
int maxMatches, ServiceItemFilter filter, long waitDur)’. I get back an
array of ServiceItem instances. The call is over. I don’t expect those
instances to be updated magically when a service updates its
registrations
in Reggie. Once I get the ServiceItems from SDM, they’re mine. I
can pass
them elsewhere or alter their values as I like. If I were to take the
Entry objects out and pass them to a different SDM call, I’d expect
the SDM
to take a snapshot of their values at the instant I make the call.
In no
case do I expect ServiceItem to be a ‘live’ object.
More complex case, I ask SDM to create a LookupCache by calling
'createLookupCache(ServiceTemplate tmpl, ServiceItemFilter filter,
ServiceDiscoveryListener listener)’…
Again, I expect SDM to snap a copy of my ServiceTemplate Entries when I
make the call. I get back a LookupCache. I call
'lookup(ServiceItemFilter
filter, int maxMatches)’ to get an array of ServiceItems that match my
original ServiceTemplate. Again, they’re not live objects; they
correspond
to a snapshot at some instant of time. I don’t even really care
about when
it is - if I don’t find a service I can use, I need to wait for a
while and
try again anyway. Who cares! Even if I do find a good-looking
service, it
might fail by the time I get around to calling it, so I’d have to
deal with
that failure, by waiting a while and doing another lookup.
If the LookupCache figures it has something to tell me, it will fire a
ServiceDiscovery event, with the pre- and post-event ServiceItem
values. I
would expect those values to not be live objects either. Personally, I
wouldn’t get too hung op on guarantees of receiving events either.
How can
SDM possibly guarantee that I get changes from Reggie, when Reggie is on
the other end of a network connection? There could always be a
temporary
failure that causes me to miss an event or two (I mean I suppose you
could
come up with some elaborate “reliable event” mechanism, but why
bother in
this case? In the end, if you can’t find a service, you wait for awhile
and do another lookup anyway). The conservative approach is to go do a
full lookup if I ever need to know the latest picture of Reggie’s state.
So I’m not quite sure what the alleged visibility issue is. When SDM or
LookupCache calls me, I’d expect the objects to be fully
constructed. If
I’m passing the ServiceItems off to another thread, I really should be
doing that inside an appropriately synchronized block, so there’s a
proper
happens-before boundary.
Is there possibility for wackiness inside ServiceDiscoveryManager?
Yes, I
would think so. But that’s not my problem as a user! If I were
going to
rewrite ServiceDiscoveryManager, I might be tempted to create objects
that
are immutable, and adopt copy-on-write semantics, but those would be
purely
implementation decisions, not API decisions. I have yet to see how the
Entry specification is affected by any concurrency issues.
Certainly, one
might want to take a look at org.apache.river.reggie.EntryRep, but Entry
objects (i.e. the public API) are fine as they are.
Cheers,
Greg Trasuk
Thanks,
Bryan
----
Bryan Thompson
Chief Scientist& Founder
SYSTAP, LLC
4501 Tower Road
Greensboro, NC 27410
br...@systap.com
http://blazegraph.com
http://blog.bigdata.com<http://bigdata.com>
http://mapgraph.io
Blazegraph™<http://www.blazegraph.com/> is our ultra high-performance
graph database that supports both RDF/SPARQL and Tinkerpop/Blueprints
APIs. Blazegraph is now available with GPU acceleration using our
disruptive
technology to accelerate data-parallel graph analytics and graph query.
CONFIDENTIALITY NOTICE: This email and its contents and attachments
are
for the sole use of the intended recipient(s) and are confidential or
proprietary to SYSTAP. Any unauthorized review, use, disclosure,
dissemination or copying of this email or its contents or
attachments is
prohibited. If you have received this communication in error, please
notify
the sender by reply email and permanently delete all copies of the
email
and its contents and attachments.
On Wed, Sep 9, 2015 at 9:00 AM, Greg Trasuk<tras...@stratuscom.com>
wrote:
On Sep 9, 2015, at 8:05 AM, Peter<j...@zeus.net.au> wrote:
It's worth noting that volatile is not excluded by the Entry
specification.
This doesn't provide any performance benefit whatsoever, but it does
provide visibility guarantees between threads. There is no atomicity…
Greg, what are your thoughts?
Entries are not intended for communication between threads. They’re
intended for communication between memory spaces across a network
boundary. Any performance difference with volatile (or even final
fields)
would be entirely masked by transit time on the network (1).
You seem to want to change the Entry specification to reflect concerns
about in-process concurrency, and I think that just fundamentally
breaks a
useful abstraction, an abstraction that is defined in the core of the
public interface to River.
I’m trying to come at this question from the point of view of a system
architect or distributed system implementer, i.e. a “user” of River,
not a
River implementer. From that perspective, what is it that you see as
“broken” about the Entry spec (
http://river.apache.org/doc/specs/html/entry-spec.html)? How does
requiring Entry fields to be final make it easier for me to build a
distributed system? Could we get opinions from Dennis? Bryan? Other
users? Their downstream users?
Cheers,
Greg
(1) - I have to state my usual caveat here - reasoning a priori about
performance in a complex system is a fool’s errand. You need to
have a
model system and use case that you can analyze and experiment on. But
I do
know we can run a whole lot of processor cycles in the time it takes
1400-or-so bytes to move across the room.
Regards,
Peter.
On 9/09/2015 7:00 PM, Peter wrote:
Greg,
Thanks for putting the time& effort to look up these links.
Primitive fields are an interesting case, I guess they had to be
disallowed, because the serial form of an int is different from an
Integer,
at least for java Serialization.
The performance trade off I mentioned relates to the time it
takes for
reflection to set each field in an Entry and when defensive copying
is
used, instead of pass by reference.
ServiceDiscoveryManager is a good example of complexity, even
after 15
years, it still contains bugs because its design is too complex (even
I'm
not 100% confident I've fixed them all).
Everything should be made as simple as possible, but not simpler.
Ken Arnold made Entry's simple for this reason, but in those
days, the
jvm still had issues with its memory model, final fields didn't
have the
thread safety guarantee they have now and most computers only had one
cpu.
https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html#finalWrong
Allowing Entry fields to be final, doesn't significantly increase
complexity, in fact, in can assist in reducing complexity in
concurrent
code.
I'm still convinced of the benefits of allowing final fields in
Entry's
:)
How about a compromise?
Leave existing Entry implementations as is, but allow final
fields in
new Entry's?
Then in River code, we treat Entry's as immutable, regardes of
whether
they are or not?
This way, existing code won't break from final modifiers, but new
code
can benefit significantly.
I'm not aware of any Entry using final fields at present.
Regards,
Peter.
On 9/09/2015 3:55 PM, Greg Trasuk wrote:
On Sep 9, 2015, at 12:30 AM, Peter<j...@zeus.net.au> wrote:
Thanks Greg,
Was it a case of; because we can't set final fields (well not
without
a Permission anyway), that they shouldn't be included in Entry
serialized
state, because then we can't deserialize them?
No, it’s just the definition of “what’s in the schema”. A class in
the normal sense is a template for objects that are going to be
instantiated and used as objects. A class that implements Entry is
not
really doing the same thing. It’s really a schema for a set of values
that
can be stored or used in a matching operation. Nothing more, nothing
less. Dynamic operations in matching are a recipe for undefined
behaviour
(e.g. changing a value that’s used as a key in a hash map), so the
Entry
spec says “don’t do that”, by requiring that the matchable fields
exist
as
fields, not calculated properties or anything else. Using getter or
setter
methods would provide an abstraction from the storage, but an
Entry is
fundamentally a storage structure, so it outlaws the abstraction. (Ken
Arnold explained this nicely in
http://www.artima.com/intv/sway2.html).
Also, a null value in a field is a “wildcard”, hence the statement
that the fields are serialized individually, so field-by-field
comparisons
are possible. Notice that if you look at the usage, the class that
implements Entry is never directly serialized - both Reggie and
Outrigger
create a representation object that has an array of serialized fields.
They are treating the Entry class as a schema definition. The Entry
instance itself is like a record formatted according to that schema.
Now, even though we’re not really going to use it as a traditional
class, certain features of classes might still be useful. Like, it
might
be useful to have constants for frequently-used values. So you can
still
have static or non-static final fields (I can’t think of a good reason
to
have non-static final fields in an Entry class, but I suppose there
might
be a use case). But any matching or storage operation only applies to
fields defined in the “schema” - by definition, the public,
non-static,
non-final fields.
I've done my best to fix the existing implementations, so
hopefully
they won't need further fixes, however, the fixes were very
difficult
and
these implementations very difficult to reason about, because there is
so
much mutable state. In ServiceDiscoveryManager, a thread holds a lock
while waiting for the result of a remote call, there was no solution I
could find to remove this lock.
To quote Keith Edwards "The Special Semantics of Attributes":
"All the methods of the object are ignored for purposes of
searching, as are "special" data fields: static, transient,
non-public, or final fields. Likewise all fields that are
primitive
types (such as ints and booleans) are ignored; only
references to
other objects within an attribute are considered for searching.”
I’ll confess, it’s always struck me as odd that primitive fields
are
ignored, and it’s certainly bit me once or twice. But I understand
that
it’s designed so that all the “matchable” fields are objects, so
can be
stored and compared in a uniform way (typically as MarshalledObjects).
Since Java5 added auto-boxing/unboxing, there’s no real coding
overhead
for
the developer.
So our choices are (for River 4.0):
1. Break backward compatibility and increase scalability,
performance
and reduce bugs, by not ignoring final fields in Entry's, but
instead mandating them.
No. That’s something else. That’s not an Entry. Such an object
might be useful for implementations, but that’s not an Entry.
An Entry is basically a local data structure that I can modify
freely. The value is crystallized when I send the Entry somewhere.
Like
to a JavaSpace. If I’m using an Entry to store things locally
where I
have concurrent access to the entry fields, that’s my problem, not the
Entry’s.
2. Or continue full compatibility and live with lower performance,
less scalability and harder to debug code.
If that tradeoff is true, you’re misusing or misunderstanding
Entries.
Not to mention, if you speed up Lookups, so what? They don’t
happen
that often - only on startup of a client or failure of a service, in
most
cases outside of the test framework. JavaSpaces are a different
issue,
but
again, without knowing the use case of the JavaSpace (messaging,
persistence, read-mostly, write-mostly, who knows?), it’s hard to
do any
reasoning about performance.
Java’s default serialization mechanism is slow? Well, I don’t know
that for sure for a given use case, but OK, in both the Registrar
and
the
JavaSpaces case, the client receives a proxy. The current proxy
implementations use JERI, but that’s an implementation detail. The
proxy
is free to implement a different serialization or communication
mechanism.
Don’t like reflection? I guess you could use ASM or BCEL to create a
marshaller object for each Entry type that a given JavaSpace
implementation
wants to support. You could probably even generate them dynamically.
For
that matter, you could precompile Google Protocol Buffers for the most
frequent Entry types. Is it worth the effort? I don’t know, but it’s
technically feasible. Jini’s dynamic code approach guarantees that
it’s
possible.
I think there's plenty of time for implementations to prepare for
River 4.0, if we start talking about it now.
Regards,
Peter.
How are these for code comments (from ServiceDiscoveryManager)?
“Those who enjoy laws or sausages shouldn’t see either being
made” -
misquoting Bismarck or Saxe. Would you rather the original
developer
didn’t document areas that she thinks might eventually cause trouble?
What can I tell you? Concurrency is hard. Distributed programming
is
hard. Hate to say it, but in a distributed scenario, it might not be
realistic to expect deterministic behaviour out of a system. Jini is
all
about embracing failure and indeterminacy. (Ken Arnold again -
http://www.artima.com/intv/distrib.html)
// Don't like the fact that we're calling
foreign code
while
// holding an object lock, however holding this
lock
doesn't
// provide an opportunity for DOS as the lock only
relates to a specific
// ServiceRegistrar and doesn't interact with
client
code.
matches = proxy.lookup(tmpl, Integer.MAX_VALUE);
/* The cache must be created inside the listener sync block,
* otherwise a race condition can occur. This is
because
the
* creation of a cache results in event registration
which
* will ultimately result in the invocation of the
serviceAdded()
* method in the cache's listener, and the
interruption of
any
* objects waiting on the cache's listener. If the
notifications
* happen to occur before commencing the wait on the
listener
* object (see below), then the wait will never be
interrupted
* because the interrupts were sent before the wait()
method
* was invoked. Synchronizing on the listener and the
listener's
* serviceAdded() method, and creating the cache only
after the
* lock has been acquired, together will prevent this
situation
* since event registration cannot occur until the
cache
is
* created, and the lock that allows entry into the
serviceAdded()
* method (which is invoked once the events do
arrive) is
not
* released until the wait() method is invoked .
*/
/**
* With respect to a given service (referenced by the
parameter
* newItem), if either an event has been received from the
given lookup
* service (referenced by the proxy parameter), or a
snapshot
of the
* given lookup service's state has been retrieved, this
method
* determines whether the service's attributes have
changed,
or whether
* a new version of the service has been registered.
After the
* appropriate determination has been made, this method
applies the
* filter associated with the current cache and sends the
appropriate
* local ServiceDiscoveryEvent(s).
*
* This method is called under the following conditions: -
when a new
* lookup service is discovered, this method will be called
for each
* previously discovered service - when a gap in the events
from a
* previously discovered lookup service is discovered, this
method will
* be called for each previously discovered service -
when a
MATCH_MATCH
* event is received, this method will be called for each
previously
* discovered service - when a NOMATCH_MATCH event is
received, this
* method will be called for each previously discovered
service Note
* that this method is never called when a MATCH_NOMATCH
event
is
* received; such an event is always handled by the
handleMatchNoMatch
* method.
*
* When this method is called, it may send one of the
following events
* or combination of events: - a service changed event - a
service
* removed event followed by a service added event - a
service
removed
* event
*
* A service removed event is sent when the service either
fails the
* filter, or the filter produces an indefinite result; in
which case,
* the service is also discarded.
*
* A service changed event is sent when the service
passes the
filter,
* and it is determined that the service's attributes have
changed. In
* this case, the old and new service proxies are
treated as
the same if
* one of the following conditions is met: - this method
was
called
* because of the receipt of a MATCH_MATCH event - the
old and
new
* service proxies are byte-wise fully equal (Note that the
lookup
* service specification guarantees that the proxies are
the
same when a
* MATCH_MATCH event is received.)
*
* A service removed event followed by a service added
event
is sent
* when the service passes the filter, and the
conditions for
which a
* service changed event would be considered are not
met; that
is, this
* method was not called because of the receipt of a
MATCH_MATCH event;
* or the old and new service proxies are not byte-wise
fully
equal.
*
* The if-else-block contained in this method implements
the
logic just
* described. The parameter matchMatchEvent reflects the
pertinent event
* state that causes this method to be called. That is,
either
a
* MATCH_MATCH event was received, or it wasn't, (and if it
wasn't, then
* a full byte-wise comparison is performed to determine
whether the
* proxies are still the same).
*
* To understand when the 'else' part of the
if-else-block is
executed,
* consider the following conditions: - there is more
than one
lookup
* service with which the service registers (ex. LUS-0 and
LUS-1) -
* after the service registers with LUS-0, a NOMATCH_MATCH
event is
* received and handled (so the service is now known to the
cache) -
* before the service registers with LUS-1, the service is
replaced with
* a new version - the NOMATCH_MATCH event resulting
from the
service's
* registration with LUS-1 is received BEFORE receiving the
* MATCH_NOMATCH/NOMATCH_MATCH event sequence that will
ultimately
* result from the re-registration of that new version with
LUS-0 When
* the above conditions occur, the NOMATCH_MATCH event that
resulted
* from the service's registration with LUS-1 will cause
this
method to
* be invoked and the proxies to be fully compared (because
the event
* was not a MATCH_MATCH event); and since the old service
proxy and the
* new service proxy will not be fully equal, the else
part of
the
* if-else-block will be executed.
*
* This method applies the filter only after the above
comparisons and
* determinations have been completed.
*/
On 9/09/2015 1:40 PM, Greg Trasuk wrote:
On Sep 8, 2015, at 10:40 PM, Peter<j...@zeus.net.au> wrote:
On 8/09/2015 11:26 PM, Greg Trasuk wrote:
That’s the current state. Changing (e.g. by enforcing a
builder
pattern or something) would add unneeded complexity for the
user if
you
ask
me.
Yes, I agree, a constructor is suitable, keep it simple.
In fact, the fact that the Entry fields are non-final is the
way
Jini knows it’s a field and not a constant. i.e. in the
AbstractEntry
docs, it specifically says "The entry fields of an Entry are its
public,
non-primitive, non-static, non-transient, non-final fields."
There's no code that inspects the field and checks whether it's
final, not within River that I'm aware of, feel free to show me
where if
I'm wrong :).
com.sun.jini.outrigger.EntryRep. Look for ‘Modifier.FINAL’ in
the
‘usableField(…)’ method. There are 12 other uses of ‘FINAL’ in
River
2.2.2. Not to mention any external product (Blitz, Rio, who knows
what)
that are designed around the Entry specification.
The EntryRep you posted below is com.sun.jini.reggie.EntryRep.
You’ll notice that it calls com.sun.jini.reggie.ClassMapper, which
also
filters final fields out of the comparisons and mappings.
The reason these fields are non final, is so they can be set
with
reflection, this is a detail of the Entry specification we
should look
at
changing.
The reason they’re non-final is so that services like Reggie and
Outrigger know what constitutes an Entry field that they might
need to
match. Entry fields are used to form the templates for matching
entries.
Entry's are not subject to the usual serialization rules. All
fields in an Entry in superclass to subclass order are stored in
an
EntryRep (appended).
At present these are set after construction using reflection,
however a constructor that accepts an array parameter, will
allow the
child
most class to pass that array up through all constructors to
reconstruct
the Entry without using reflection (magnitudes faster performance
wise).
Eliminating the use of reflection during deserialization will
increase performance, and immutability increases scalability.
Where do we use entries? Typically in doing Service
registrations
and lookups. Most users can understand that the Entry is going to be
serialized and used remotely.
Clients of Javaspaces and utility classes too, like JoinManager
and
ServiceDiscoveryManager.
