Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable Discovery Mode #23

Closed
smcvb opened this issue May 6, 2020 · 1 comment · Fixed by #28
Closed

Configurable Discovery Mode #23

smcvb opened this issue May 6, 2020 · 1 comment · Fixed by #28

Comments

@smcvb
Copy link
Member

smcvb commented May 6, 2020

As time has proven the original approach of the SpringCloudCommandRouter, thus using ServiceInstance metadata to communicate command handling capabilities, no longer suffices for a multitude of Spring Cloud Discovery mechanism implementations. This follows from the fact that as far as we know currently no implementation allows continued updates on this metadata in a live set up.

That leaves the SpringCloudHttpBackupCommandRouter as the sole reasonable solution, with a rather awkward naming attached to it. As it's no longer a back-up, but the only solution.
We feel an overhaul on the SpringCloudCommandRouter is thus in place.

Instead of the current set up we should introduce the capability to configure a (for a lack of better word) DiscoveryMode on the SpringCloudCommandRouter.

This DiscoveryMode would provide the implementation on how to receive and send command routing information from one instance to another. We think the following implementations would be reasonable to introduce:

@dhermanns
Copy link
Contributor

Sounds great! Thanks for picking that up!

@smcvb smcvb added this to the Release 4.4 milestone Aug 5, 2020
@smcvb smcvb self-assigned this Aug 14, 2020
smcvb added a commit that referenced this issue Aug 21, 2020
Introduce the CapabilityDiscoveryMode which is used to retrieve a nodes
command handling capabilities, and to discovery other nodes their
capabilities with. Consolidate these capabilities in a
MemberCapabilities object

#23
smcvb added a commit that referenced this issue Aug 21, 2020
Introduce a default implementation, storing the load factor and
CommandFilter as is. Next to that, introduce a implementation which
serializes the CommandMessageFilter to be sent over the wire

#23
smcvb added a commit that referenced this issue Aug 21, 2020
Build a layered CapabilityDiscoveryMode implementation which uses the
REST protocol to share and retrieve MemberCapabilities. Additionally,
let it allow wrapping logic to ignore certain instances if they have
thrown a ServiceInstanceClientException exception.

#23
smcvb added a commit that referenced this issue Aug 21, 2020
Create a simple CapabilityDiscoveryMode implementation, which can handle
 everything and does not add instances to the ignore list. This
 implementation can be used for homogenous Axon set ups.

#23
smcvb added a commit that referenced this issue Aug 21, 2020
Fix camelcasing in the contextRootMetadataPropertyname field and
parameter

#23
smcvb added a commit that referenced this issue Aug 21, 2020
Adjust the SpringCloudCommandRouter to utilize the new
CapabilityDiscoveryMode. The current solution breaks backwards
compatability though, definitely when it comes to the
SpringCloudHttpBackupCommandRouter.

#23
smcvb added a commit that referenced this issue Oct 12, 2020
Rename package from capabilitydiscoverymode to mode, to be more concise.

#23
smcvb added a commit that referenced this issue Oct 12, 2020
Adjust the indention of the javadoc section according to the style guide

#23
smcvb added a commit that referenced this issue Oct 12, 2020
Introduce JUnit 5 to be used during the test cases

#23
smcvb added a commit that referenced this issue Oct 12, 2020
Add test cases for the current stance of all new components added to the
 springcloud.commandhandling.mode package

#23
smcvb added a commit that referenced this issue Oct 19, 2020
Update the test cases for the SpringCloudCommandRouter to follow the new
 paradigm using the CapabilityDiscoveryMode, as well as JUnit5.
Additionally, introduce a TestSerializer building a secure
XStreamSerializer instance to remove some warn logs. Lastly, the
 SpringCloudCommandRouter should be reindented, reordered and cleaned of
 any warnings.

#23
smcvb added a commit that referenced this issue Dec 9, 2020
Add specifics about the generic parameter to the builder's javadoc

#23
smcvb added a commit that referenced this issue Dec 22, 2020
Even though MessageRoutingInformation and MemberCapabilities resemble
each other quite a lot, we'll move to the newer variant. Partially
because it's a move from implementation to interface, but also due to
the deprecation of the SpringCloudHttpBackupCommandRouter which is the
sole user of the MessageRoutingInformation. MemberCapabilities on the
other hand is core to the capability discovery mode implementations

#23
smcvb added a commit that referenced this issue Dec 22, 2020
- Switch Assert import statement
- Remove use of MockitoExtension

#23
smcvb added a commit that referenced this issue Dec 22, 2020
The entire SpringCloudHttpBackupCommandRouter should be deprecated, but
should also still be usable. Towards that end, all overridden methods
have been removed from the CommandRouter, deprecating the
getLocalMessageRoutingInformation further more and defaulting it to a
none-usable MessageRoutingInformation instance. The builder of the
CommandRouter should default to a RestCapabilityDiscoveryMode instance
which is ignore-listing instances, as that was the original approach of
this router. Any REST specific annotations should also be removed from
this Router impl. The javadoc should correctly reflect the left
over intent of this router. Lastly, the existing test cases should be
validated for the worth and copied over to the respective replacement
discovery mode implementations (if applicable).

#23
smcvb added a commit that referenced this issue Dec 22, 2020
Rename messageRoutingInformationEndpoint to messageCapabilitiesEndpoint
to better reflect it's intent

#23
smcvb added a commit that referenced this issue Dec 22, 2020
Update the SpringCloudAutoConfiguration according to the introduction
of CapabilityDiscoveryModes. To that end the auto-config should no
longer create a SpringCloudHttpBackupCommandRouter, but only a
SpringCloudCommandRouter. Furthermore, it would be beneficial if the
property files allow for configuring all possible types of
CapabilityDiscoveryMode on the settings.

#23
smcvb added a commit that referenced this issue Dec 22, 2020
Remove resolved todos, or todos which will be covered on a different
level

#23
smcvb added a commit that referenced this issue Jan 6, 2021
- Add exception the log message
- Switch fallback to rest-mode in @RequestMapping annotation
- Move creation of the RestCapabilityDiscoveryMode to the start of the
validate method
- Remove useless suppressed warning

#23
smcvb added a commit that referenced this issue Jan 6, 2021
Adjust Axon version to 4.4.4

#23
smcvb added a commit that referenced this issue Jan 6, 2021
The SerializedMemberCapabilities should be the returned object from
within a RestCapabilityDiscoveryMode implementation. This also means it
should be serializable by a RestTemplate's default settings.

#23
smcvb added a commit that referenced this issue Jan 6, 2021
By automatically generating a delegate RestCapabilityDiscoveryMode
inside an IgnoreListing instance, the capability is lost to
automatically register the Rest-version as a RestController. As such,
GET operations don't work. Instead the wrapping functionality should be
done in the auto configurer based on the property, right when the
CommandRouter is built.

#23
smcvb added a commit that referenced this issue Jan 7, 2021
Adjust the SerializedMemberCapabilities to not be an implementation of
MemberCapabilities. Instead, it's simply a holder of the serialized
MemberCapabilities to be send over the wire. On the receiving end, we
simply push the SerializedMemberCapabilities into a
DefaultMemberCapabilities through it's distinct constructor taking in
the serialized format and a serializer

#23
smcvb added a commit that referenced this issue Jan 7, 2021
Correct javadoc to not be specific about HTTP exception, as it's about
general ServiceInstance exceptions thrown when the application tries to
retrieve MemberCapabilities

#23
smcvb added a commit that referenced this issue Jan 7, 2021
Adjust the SpringCloudHttpBackupCommandRouter to return actual working
MessageRoutingInformation. As such, it should be the GetMapping again,
reusing the old request mapping. This should work as the serialized
format of the MessageRoutingInformation and SerializedMemberCapabilities
 is identical. Lastly, wrap the RestCapabilityDiscoveryMode in an ignore
 instance, as that used to be the behavior.

#23
smcvb added a commit that referenced this issue Jan 7, 2021
The auto config property should speak about enabling ignore listing
instead of disabling, as this clarifies the intent. Furthermore, rename
the IgnoreListingCapabilityDiscoveryMode instances to
IgnoreListingDiscoveryMode to simplify the name

#23
smcvb added a commit that referenced this issue Jan 7, 2021
The SimpleCapabilityDiscoveryMode isn't clarifying the intent of this
implementation. It should instead be called
AcceptAllCommandsDiscoveryMode, as that's what it does. Furthermore, it
 should not be a hard implementation of the RestCapabilityDiscoveryMode,
 but rather use a delegate CapabilityDiscoveryMode. This switches it to
 a decorator i.o. an full implementation, which is fine for it's use
 case. Reflect this in the auto configuration, by removing the SIMPLE
 mode in favor of a property. Lastly, the decorators (ignore listing and
  accept-all) should be wrapped around another CapabilityDiscoveryMode
  instance.

#23
smcvb added a commit that referenced this issue Jan 7, 2021
Drop reference of Serializer, as it's not used in this impl.

#23
smcvb added a commit that referenced this issue Jan 7, 2021
Insert the builder paradigm for IgnoreListingDiscoveryMode for added
flexibility

#23
smcvb added a commit that referenced this issue Jan 7, 2021
Set up the auto config such that a RestCapabilityDiscoveryMode is only
created when the mode==REST. Furthermore, let it be decorated with the
AcceptAllCommand and/or IgnoreListing discovery mode if either of both
have been enabled.

#23
smcvb added a commit that referenced this issue Jan 7, 2021
When creating the RestCapabilityDiscoveryMode in a Spring environment,
it is a hard requirement to be a bean, because otherwise it's controller
 capabilities will not be picked up automatically. If the bean is
 pre-wrapped (e.g. AcceptAll/IgnoreListing) however, this auto discovery
 capability is lost. When can cover for this in the auto-config, but
 users can still supply their own beans if they want. This issue can be
 solved by introducing a dedicated MemberCapabilitiesController which
 takes in the RestCapabilityDiscoveryMode for the purpose of sharing a
 members capabilities.

#23
smcvb added a commit that referenced this issue Jan 8, 2021
Upgrade to framework 4.4.5 and spring cloud Hoxton.SR9

#23
smcvb added a commit that referenced this issue Jan 8, 2021
If the DiscoveryClient does not return any ServiceInstances, an update
of the consistent hash would mean nobody can handle anything. In reality
 though, the node itself can typically still handle something. Hence the
 localServiceInstance should be included if the instances set is empty.
Note that not doing this is an issue when testing with a Eureka
discovery service. Right after the InstanceRegisteredEvent we clear out
the local instance to correctly reflect the URI it should at that point
at least have. The DiscoveryClient at that stage however still doesn't
know anything, not even the local registration. Hence the consistent
hash could incorrectly be cleared out in such a scenario, which is thus
covered by adding it if no services are found.

#23
smcvb added a commit that referenced this issue Jan 8, 2021
Make dedicated testSubject outside the lambda so that SonarCloud doesn't
 complain about the complexity

#23
smcvb added a commit that referenced this issue Jan 8, 2021
Fix a bunch of javadoc inconsistencies in all the adjusted classes. Next
 to that the IgnoreListing and AcceptAllCommands CapabilityDiscoveryMode
 implementations can extend the AbstractCapabilityDiscoveryMode for
 consistency. Lastly, the Serializer is not
 AbstractCapabilityDiscoveryMode but Rest specific (for now), hence it
 should be placed there.

#23
@smcvb smcvb closed this as completed in #28 Jan 15, 2021
smcvb added a commit that referenced this issue Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants