Move membership code to a separate gradle sub-project

Authors: Dan Smith, Bruce Schuchardt

Status: Development

Superseded by: N/A

Related: N/A

Problem

Geode has its own custom membership system, which is responsible for discovering other members of the system and for detecting and removing failed members. We want to be able to exhaustively test that the membership system is working correctly in all circumstances. However this membership system is not cleanly separated from the rest of the geode code. This makes it difficult and expensive to test, because we have to create full geode members and connect them over a network in order to test any interaction between the membership system of two geode members.

By isolating the membership code such that it does not depend on the rest of the geode, we can test the membership system by itself. This will allow us to write much faster and more exhaustive tests of how multiple members interact.

Creating a well defined internal API for the membership system and hiding the internals from the rest of geode will also make it easier for geode developers to reason about what the membership system is and is not doing while writing and testing other components of geode.

Anti-Goals

  • It is not a goal to give the membership system a public API for use by geode users. The membership system will have an internal  API and be usable in isolation, but we don't intend to advertise it as a publicly available component by itself.
  • It is not a goal to create an SPI that allows different membership systems to be swapped in.
  • It is not a goal to change the membership algorithms or wire protocol in any way.

Solution

We will create a new gradle subproject called geode-membership. All of the code in the org.apache.geode.distributed.internal.membership.gms and related unit tests will be moved out of geode-core and into geode-membership. geode-core will have a dependency on geode-membership, and interact with membership through its API, defined below.

geode-membership itself depends on serialization. Therefore we will also create a new gradle subproject for serialization, called geode-serialization. The full dependency tree will look something like below.

geode-core geode-membership geode-serialization geode-tcp-server



Breaking circular dependencies


The membership code currently has a number of dependencies on other classes within geode core. We will break these dependencies in essentially two different ways:

  1. Move the dependencies to a separate subproject - serialization and tcp-server (Locator infrastructure) are moving to separate subprojects so that membership can depend directly on them
  2. Inject dependencies at runtime - dependencies such as gemfire stats can be injected into the membership system by providing interfaces such as MembershipStatistics (see below) that geode core must provide when creating the membership system.

Classes used to wrap the membership APIs or inject dependencies into membership will be put in the org.apache.geode.distributed.internal.membership.adapter package.

Delineating Interfaces

Not only does this RFC specify inter-subproject dependency constraints (between geode-core, geode-membership, geode-serialization), it goes further. Classes outside geode-membership may only see geode-membership interfaces defined in the (TBD) membership API package (see below). Other interfaces and classes defined by membership will be inaccessible outside of membership.

Gradle Subprojects First—Java Modules Someday

This RFC describes creation of (2) new Gradle subprojects (geode-membership, geode-serialization). It also specifies the the allowed (and disallowed) inter-subproject dependencies. Our intention is to lay the ground for eventual use of Java modules to enforce these dependency constraints.

geode-membership API

The geode-membership subproject will provide the following API to the rest of the system. We will enforce that other parts of the system can only interact with this API.


MembershipBuilder static newMembershipBuilder() set...() Membership Core class of a running membership system. getMembershipView(): MembershipView Messenger send(Message) MembershipView getMembers(): List<MemberIdentifier> MemberIdentifier A single member of the system MembershipListener MessageHandler Authenticator MembershipStatistics Config creates

/** 
* Creates the membership system, given the provided configuration
*/

interface MembershipBuilder {
  static MembershipBuilder newMembershipBuilder()
  MembershipBuilder setConfig(Config)
  MembershipBuilder setAuthenticator(Authenticator)
  MembershipBuilder setMembershipListener(MembershipListener)
  MembershipBuilder setMessageHandler(MessageHandler)
  MembershipBuilder setMembershipStatistics(MembershipStatistics)
  MembershipBuilder setMemberIDFactory(MemberIdentifierFactory)
  create() : Membership
}

/**
* Core class of a running membership system.
*/
interface Membership {
  getMembershipView(): MembershipView
  getMessenger(): Messenger
  getLocalMember(): MemberIdentifier
  contactedBy(MemberIdentifier)
  isShunned(MemberIdentifier)
  close()
  isClosed()
}

/**
* API for sending messages to other members, using memberships messaging system
* Membership currently allows geode-core to send messages over it's UDP messaging system. 
* This interface provides that functionality.
*/
interface Messenger {
  send(Message)
  /** Get an object the represents what messages have been sent to to the given
   * MemberIdentifier. This is used in waitForMessengerState to wait for these messages
   * to arrive on the remote side           
   */  
  getMessengerState(MemberIdentifer): MessengerState
  waitForMessengerState(MessengerState)
}

/**
 * A Message that will be sent to the returned recipients.
 */
interface Message {
  List<MemberIdentifier> getRecipients()
}


/**
* An object which represents the state of the sending side of a communication
* channel at a point in time. This object can be passed to the waitForMessengerState
* method on the receiving side to ensure all messages have been received on
* the receiving side
*/
interface MessengerState extends DataSerializableFixedID {
}/** 
* Interface used to create a MemberIdentifier from a set of properties
* This API is provided solely to allow geode-core to add additional
* properties to the MemberIdentifier that are not relevant to membership
* For example DurableClientAttributes. Membership will provide
* A default MemberIdentifier which just has the membership relevant attributes
*/

interface MemberIdentifierFactory {
  MemberIdentifier create(memberData)
}

/**
* A single member of the system. In practice, this will
* be implemented by InternalDistributedMember
*/
interface MemberIdentifier {
}

/**
* Provides the current members of the system
*/
interface MembershipView {
  List<MemberIdentifier> getMembers()
}

/**
* Receives notifications about changes to membership
*/
interface MembershipListener {
  memberJoined(MemberIdentifier)
  memberDeparted(MemberIdentifier)
  memberCrashed(MemberIdentifier)
  forceDisconnected(String reason)
}

/**
* Receives all messages sent from other members
*/
interface MessageHandler {
  processMessage(Message)
}

/**
* Interface used by membership to authenticate other members
*/
interface Authenticator {
/**
   * Authenticate peer member
   *
   * @return null if authentication succeed (including no authenticator case), otherwise, return
   *         failure message
   */
  String authenticate(MemberIdentifier member, Properties credentials);

  /**
   * Get credential object for the given GemFire distributed member.
   */
  Properties getCredentials(MemberIdentifier member);
}

/**
* Interface to notify statistics systems about membership changes
*/
interface MembershipStatistics {
  memberJoined()
  memberDeparted()
  ...
}

/**
* Primitive configuration options for membership
* - timeouts, etc.
*/
interface Config {
  getJoinTimeout()
  ...
}




geode-serialization API

The geode-serialization subproject will provide the following API to the rest of the system. We will enforce that other parts of the system can only interact with this API.

Internally Geode uses DataSerializableFixedID as the interface for most messages and data objects.  The membership system uses this exclusively.  We will separate the serialization framework for this interface and leave the rest (PDX, DataSerializable, etc.) in geode-core.

DataSerializableFixedID currently has the same signature for its toData and fromData methods but this will be changed to take another parameter, a serialization context.  The context provides access to serializers and information about the source/destination (currently the peer's version).


/** Use DSFIDSerializerFactory to create a serializer */
class DSFIDSerializerFactory {
  /** set an ObjectSerializer that should be used when invoking
   *  toData/fromData methods on objects.  This will typically
   *  defer serialization of DataSerializableFixedID objects to
   *  the DSFIDSerializer
   */
  DSFIDSerializerFactory setObjectSerializer(ObjectSerializer serializer);
  DSFIDSerializerFactory setObjectDeserializer(ObjectDeserializer deserializer);
  DSFIDSerializer create();
}

/** writes and reads DataSerializableFixedID instances.  Supports reading/writing of nulls */
interface DSFIDSerializer {
  /** retrieve the ObjectSerializer.  If none was given to the factory this will be
   *  a default serializer that can only handle DataSerializableFixedID objects */
  ObjectSerializer getObjectSerializer();
  ObjectDeserializer getObjectDeserializer();

  /** register a class with the serializer.  It must have a no-arg constructor */
  void registerDSFID(int dsfid, Class dsfidClass);

  SerializationContext createSerializationContext(DataOutput dataOutput);
  DeserializationContext createDeserializationContext(DataInput dataInput);

  /** write a DataSerializableFixedID to the given DataOutput.  This method should be used
   *  by extensions to DSFIDSerializer, like InternalDataSerializer, that want to handle
   *  more types in ObjectSerializer but defer DSFID serialization to the
   *  default implementation */
  void writeDSFID(DataSerializableFixedID o, int dsfid, DataOutput out) throws IOException;

  /** read a DataSerializableFixedID from the given DataInput.  This method uses constructors
   *  registered via registerDSFID() and should be used by extensions to DSFIDSerializer,
   *  like InternalDataSerializer, that want to handle more types in ObjectDeserializer but
   *  defer DSFID serialization to the default implementation */
  Object create(int dsfid, DataInput in) throws IOException, ClassNotFoundException;

  void invokeToData(Object ds, DataOutput out) throws IOException;
  void invokeFromData(Object ds, DataInput in) throws IOException, ClassNotFoundException;

  int readDSFIDHeader(DataInput dis) throws IOException;
  void writeDSFIDHeader(int dsfid, DataOutput out) throws IOException;
}

 /** provides context for toData serialization methods */
  interface SerializationContext {
    ObjectSerializer getSerializer();
    Version getSerializationVersion();
  }

  /** provides context for toData serialization methods */
  interface DeserializationContext {
    ObjectDeserializer getDeserializer();
    Version getSerializationVersion();
  }

  interface ObjectSerializer {
    public void writeObject(Object obj, DataOutput output) throws IOException;
    void invokeToData(Object ds, DataOutput out) throws IOException;
    ...
  }

  interface ObjectDeserializer {
    public Object readObject(DataInput input) throws IOException, ClassNotFoundException;
    void invokeFromData(Object ds, DataInput in) throws IOException, ClassNotFoundException;
    ...
  }

/** a new superclass of HeapDataOutputStream that separates the methods used with Buffers from
    a bunch of other methods concerning other functionality such as Streamables, PDX, etc. */
class BufferDataOutputStream implements DataOutputStream {
  // methods moved from HeapDataOutputStream unchanged
}

/** DataSerializableFixedID is modified to have more parameters in toData/fromData methods */
interface DataSerializableFixedID {
  public Version[] getSerializationVersions();
  public int getDSFID();
  public void toData(DataOutput output, SerializationContext context) throws IOException;
  public void fromData(DataInput input, DeserializationContext context) throws ClassNotFoundException, IOException;
}


A collection of other classes will be moved, virtually unchanged, into the new sub-project to fill out the API:

  • ByteArrayDataInput
  • DataSerializableFixedID will continue to hold all of Geode's DSFID constants until we find a better home for them.
  • DSCODE - basic serialization constants
  • DscodeHelper
  • DSFIDNotFoundException
  • SerializationVersions - interface for backward-compatible serialization
  • ThreadLocalByteArrayCache
  • VersionedDataInputStream
  • VersionedDataOutputStream
  • VersionedDataStream
  • Version - Geode's serialization version class

Use DSFIDSerializerFactory to create a new serializer.  Set an ObjectSerializer/ObjectDeserializer if you want to provide serialization of other types of objects in readObject/writeObject.  Use DSFIDSerializer.registerClass() to register your classes so that the serializer knows how to instantiate them.

The new DSFID serializer sub-project will by and large not be composed of static methods as is InternalDataSerializer and its superclass DataSerializer.  InternalDataSerializer will hold an instance of the DSFIDSerializer and will register DSFID codes and constructors with it.  Instantiation will insert an ObjectSerializer and ObjectDeserializer into the DSFIDSerializer to provide users the ability to write PDX, DataSerializables and other serialization types supported by Geode but not (directly) by the DSFIDSerializer.  InternalDataSerializer and DataSerializer will defer serialization of DSFIDs to the DSFIDSerializer instance.  The geode-core DSFIDFactory will register all geode-core DataSerializableFixedID classes with the instance of the serializer held by InternalDataSerializer, as will other Geode subprojects like geode-wan.

While serialization of primitives and Strings can be performed using DataInput and DataOutput there are a number of static serialization methods currently in geode-core that will have to be shifted into the DSFIDSerializer (writeString/readString, writeHashmap/readHashmap, etc) to maintain compatibility with past releases.  Those methods shifted from DataSerializer will remain available in that class since it is a public API.   The intent is to shift these methods further into the ObjectSerializer/ObjectDeserializer APIs and stop using static serialization methods.

Since Version is being repackaged any rolling-upgrade test code that refers to it will no longer work when running with an older version of Geode.  We will provide methods in the distributed test framework VersionManager to make this less painful.  We will replace the use of Version.CURRENT_ORDINAL with VersionManager.getInstance().getCurrentVersionOrdinal() in these tests.  Other methods will be added to VersionManager as needed.


Changes and Additions to Public Interfaces

This proposal does not add or remove any public API.

Performance Impact

The intention of this proposal is to not change the performance of geode significantly.

Backwards Compatibility and Upgrade Path

These changes will maintain backwards compatibility with rolling upgrades. No messages will change as a result of this proposal.

This change does introduce new geode-membership and geode-serialization maven artifacts. Users building a classpath with maven/gradle/etc. or using gfsh will not be impacted. However, if someone is manually launching a process with a hardcoded list of geode jars, they will need to add the new geode-membership  and geode-serialization jars.

Prior Art


There have been multiple proposals to make geode more modular  - Geode Modularization Proposal (work in progress), Proposal for Geode Modularization. This proposal can be considered an incremental step in that direction.

As an alternative, we could isolate the membership code without creating a new gradle subproject and geode-membership jar file. However without enforcing what the membership code can and cannot depend on, it is likely that the membership code will not stay isolated as different developers work on the code.


FAQ

Q: Why not introduce a separate messaging module, and leave things like Messenger out of geode-membership?

A: We may eventually try to split messaging out of membership, but that is not part of the current proposal. The reason membership includes messaging in this proposal is that the product currently uses the messaging system bootstrapped by membership to send arbitrary messages. Membership requires a lower level messaging system (jgroups) but it uses that to provide a higher level messaging system to the rest of the system. The higher level messaging provided by membership includes additional features such as cluster wide encryption and ensuring that we only process messages from current members.

Errata

What are minor adjustments that had to be made to the proposal since it was approved?

  • No labels

33 Comments

  1. send(Message)
    I think we need a definition for Message.
    for getMessageState()/waitForMessageState() I think we need to say more about the message state Object.
  2. forceDisconnected()
    I think forceDisconnect() should take an argument that describes why this is being invoked.  The shutdown code in GemFireCacheImpl will want to know why a force-disconnect happened.
  3. List<MemberID> getMembers()
    We could provide other information in the membership view like who created it, what's its view ID, what members shut down in this view, what members were declared crashed in this view.
    1. I think we should only provide the information that users of this API need to know about. Is this information important to expose to consumers of this API?

  4. interface MembershipStatistics { memberJoined() memberDeparted()
    add "..." to this list
  5. interface MembershipListener { memberJoined(MemberID) memberDeparted(MemberID)
    add memberCrashed(MemberID)
  6. is getMessageState() and waitForMessageState() returning the state for only the last message to be sent through the send() api?  Just wanted to confirm because the comment reads :" Get an object the represents the messages ..."  If so, maybe something like "Get an object that represents the last message sent..."

    Will there be more info for Config and Authenticator interfaces?  Authenticator is empty and Config has a ... in the interface description

    Version is the only class getting a new package structure?  

    Is the long term plan to move pdx and such into the serialization module or will those themselves be extracted into new/different modules?

    1. getMessageState() returns the state of the communications channel, which represents all of the messages sent. Internally, I think it will just be a counter of the number of messages sent or something like that.

      Config is primitive configuration options. I'm not sure exactly what all of them will be, but there will likely be quite a few - ports, timeouts, hostnames, etc. You can see what is on the existing ServiceConfig class in geode for an idea.

      Authenticator - fixed that. We actually already have this interface in geode.

      I think PDX will be a different module long term - it has more dependencies than this primitive serialization module.

  7. Not familiar enough to comment on implementation details but +1 on the overall initiative. 

  8. In the interface definition of the Builder:

    • Why is a static method required to create a builder. It seems that we are mixing what Builders and Factories are supposed to do. Why not keep the interface clean. The create method invoke the create method on a factory, which will create the correct `Membership` given the builder properties and `MembershipAlgorithm`.
    interface MembershipBuilder {
      setMembershipAlgorithm(MembershipAlgorithm) : MembershipBuilder
    }


    • Imo, the builder's setters should always return the builder again.
    interface MembershipBuilder {
      setConfig(Config) : MembershipBuilder
      setAuthenticator(Authenticator) : MembershipBuilder
      setMembershipListener(MembershipListener) : MembershipBuilder
      setMessageHandler(MessageHandler) : MembershipBuilder
      setMembershipStatistics(MembershipStatistics) : MembershipBuilder
      setMemberFactory(MemberIDFactory) : MembershipBuilder
      create() : Membership
    }



    1. I agree with you that the static method seems a bit superfluous now.

      With regards to your second comment, I think we were excluding return types from the set methods because there are different ways to implement a builder, some of which do not return the original builder type from each set method.  It is also possible to return intermediate types and use static typing to enforce contracts via a sort of type-safe DSL.  That being said, for the RFC I don't see a problem with proposing the standard builder pattern you suggested, though we might tweak it down the line if necessary.  I'll let others reply before making the change though in case anyone disagrees.

    2. @udo the Builder is to be implemented as you discussed, we'll get the diagram updated, thanks.

  9. It seems odd that getMessageState and waitForMessageState work with "Object". Should we have a MessageState interface that is used here? Also it sounds like you would call getMessageState in one member and then somehow transport that Object to another member so that you could call waitForMessageState. So does the Object need to be serializable?

    Is it really MessageState or should it be MessengerState? The description you have makes it sound like it has to do with the state of the messenger not a particular message.

    1. I added a MessengerState interface. I marked it as DataserializableFixedId. Let me know what you think.

  10. Is the name "geode-serialization" misleading since most of the code that geode user will use for serialization (Pdx, DataSerializable) are still in the core?

    1. Is the intent to eventually move the other serialization components to geode-serialization?

      1. There's been a long-standing desire to move more of serialization into another sub-project that isn't dependent on geode-core and I was in on most of the discussion around doing this.  Moving DSFID serialization is a first step that gives us geode-core-independent serialization for the new membership sub-project.

        1. If this is the first step to move serialization out of core and into an independent module, why is there not a consistent "Serializer" interface that the now extracted DSFID-serializer would implement?

          I understand that we cannot boil the ocean, but could we not take a little more time, come up with a "standard" serialization interface that we could use for all serializers (PDX,DataSerializable,....).

          That way we can decouple the explicit "DSFIDSerializer" for membership to something a little more abstract.

          This pattern can then be applied to future rework efforts to extract PDX and DataSerializable out of the core.

          1. The interfaces provided are ObjectSerializer and ObjectDeserializer.  The DSFIDSerializer has basic implementations of these that handle DSFID objects.  Geode-core will inject its own implementations that use DataSerializer writeObject/readObject, so it can handle DataSerializables and PDX.

  11. will methods like writeString that have to remain on DataSerializer be able to defer to the new writeString you add to this project or will the code need to be copied into both projects?

    1. yes, the methods in geode-core will just invoke the corresponding methods in geode-serialization

  12. +1

    I'm happy to see us move toward better testability for the membership code!

  13. I'm confused..

    Why is this factory required?

    class DSFIDSerializerFactory {
      /** set an ObjectSerializer that should be used when invoking
       *  toData/fromData methods on objects.  This will typically
       *  defer serialization of DataSerializableFixedID objects to
       *  the DSFIDSerializer
       */
      DSFIDSerializerFactory setObjectSerializer(ObjectSerializer serializer);
      DSFIDSerializerFactory setObjectDeserializer(ObjectDeserializer deserializer);
      DSFIDSerializer create();
    }


    Why would is not be good enough to just have constructor? Do we envision that there would be more than 1 type of DSFIDSerializer?

  14. void writeDSFID(DataSerializableFixedID o, int dsfid, DataOutput out) throws IOException;
    void writeDSFID(DataSerializableFixedID dsfid, DataOutput out) throws IOException;

    Does the first method signature mean that we can serialize a DataSerializableFixedID instance with a different id? Do we have a similar mapping on the reading side?

    1. It's just an implementation wart that we think will go away once the bulk of this RFC is implemented.

      1. Bruce J Schuchardt - maybe we can remove these "warts" from the RFC then - this doc should have as close to the final/ideal state of the API as we can get.

  15. Why do we need the DataInput / DataOutput on the following code? The create instances do not even store / use them (at least not from what I can see)

    SerializationContext createSerializationContext(DataOutput dataOutput);
    DeserializationContext createDeserializationContext(DataInput dataInput);
    1. The serialization context needs the DataOutput/DataInput in order to discern its serialization version.

      1. Do even need createSerializationContext on the DSFIDSerializer? Shouldn't writeDSFID just create the serialization context underneath the covers?

        Also, maybe we should only support VersionedDataInput/VersionedDataOutPut?  Otherwise we are doing smelly checks on the type of the DataInput we have.