search

Terracotta-OSS / terracotta-apis

Apache License 2.0
6 stars 25 forks source link

Serializability of the data passed to IMonitoringProducer must be defined #135

Closed jd0-sag closed 8 years ago

jd0-sag commented 8 years ago

Currently, IMonitoringProducer accepts data in the form of Object instances. This works fine for objects which never leave the address space of the JVM. However, this limits how the data can be used. Specifically, it prevents the platform from being able to investigate ways of passing this data between servers on the same stripe, which will likely be required for effective monitoring of passive server state and entities.

Here are 3 suggestions for how to address this, to get the conversation started:

  1. Change the value type from Object to Serializable. This is simple but does fix the serialization approach which may appear sub-optimal to some use-cases.
  2. Change the value type from Object to byte[]. This maximizes the freedom of the serialization approach but pushing this burden into the services and entities does make their implementation quite cumbersome.
  3. Require that a codec is passed into the monitoring service when it is looked up (as part of its ServiceConfiguration). This works well on the encoding side but not so well on the decoding side where it isn't clear which codec would be used (although it may be possible to assume they are the same, between servers, for a given consumer ID).
jd0-sag commented 8 years ago

@mathieucarbou @anthonydahanne @myronkscott @ramsai1729: Let me know what you think.

mathieucarbou commented 8 years ago

I would be in favor of 1. As we discussed, this seems to be the easier one and then the platform would be responsible for the serialization, the way it wants to do it.

anthonydahanne commented 8 years ago

I also like 1 for its simplicity. As a voltron user, 2 does not sound appealing :-) if 3 was chosen I'm pretty sure we would use a VoltronProxy-like layer to handle and keep in sync the codecs across nodes

jd0-sag commented 8 years ago

For the most part, this change to Serializable is relatively straight-forward. The exception to this appears to be in PlatformClientFetchedEntity since there is a ClientDescriptor as part of it.

My current thinking is to make this a transient instance variable for 2 reasons:

  1. This is somewhat similar to the idea of a file descriptor in that deserializing it doesn't mean that you can use it in any meaningful or safe way
  2. The only reason why we are likely to serialize this object (and this MUST be documented in the class) is if we are passing information from the passive back to the active and PlatformClientFetchedEntity instances aren't created on the passive (meaning that this is well-defined way of limiting the scope of what is probably a moot point)