Skip to content

Conversation

@s1monw
Copy link
Contributor

@s1monw s1monw commented May 1, 2017

TransportService and RemoteClusterService are closely coupled already today
and to simplify remote cluster integration down the road it can be a direct
dependency of TransportService. This change moves RemoteClusterService into
TransportService with the goal to make it a hidden implementation detail
of TransportService in followup changes.

TransportService and RemoteClusterService are closely coupled already today
and to simplify remote cluster integration down the road it can be a direct
dependency of TransportService. This change moves RemoteClusterService into
TransportService with the goal to make it a hidden implmementation detail
of TransportSerivce in followup changes.
Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a comment or two, LGTM otherwise

false, false,
(request, channel) -> channel.sendResponse(
new HandshakeResponse(localNode, clusterName, localNode.getVersion())));
if (remoteClusterService != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of curiosity, when can it be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it used to be in an earlier commit... I will remove

clusterSettings.addSettingsUpdateConsumer(TRACE_LOG_EXCLUDE_SETTING, this::setTracerLogExclude);
if (connectToRemoteCluster) {
clusterSettings.addAffixUpdateConsumer(RemoteClusterService.REMOTE_CLUSTERS_SEEDS,
remoteClusterService::updateRemoteCluster, (namespace, value) -> {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be calling listenForUpdates here ? where is it called now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I didn't update after I merged...

@s1monw s1monw merged commit 2f9e946 into elastic:master May 2, 2017
s1monw added a commit that referenced this pull request May 2, 2017
TransportService and RemoteClusterService are closely coupled already today
and to simplify remote cluster integration down the road it can be a direct
dependency of TransportService. This change moves RemoteClusterService into
TransportService with the goal to make it a hidden implementation detail
of TransportService in followup changes.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 2, 2017
* master: (27 commits)
  Check index sorting with no replica since we cannot ensure that the replica index is ready when forceMerge is called. Closes elastic#24416
  Docs: correct indentation on callout
  Build that java api docs from a test (elastic#24354)
  Move RemoteClusterService into TransportService (elastic#24424)
  Fix license header in WildflyIT.java
  Try not to lose stacktraces (elastic#24426)
  [DOCS] Update XPack Reference URL for 5.4 (elastic#24425)
  Painless: Add tests to check for existence and correct detection of the special Java 9 optimizations: Indified String concat and MethodHandles#ArrayLengthHelper() (elastic#24405)
  Extract a common base class to allow services to listen to remote cluster config updates (elastic#24367)
  Adds check to snapshot repository incompatible-snapshots blob to delete a pre-existing one before attempting to overwrite it.
  Added docs for batched_reduce_size
  Fixes checkstyle errors
  Allow scripted metric agg to access `_score` (elastic#24295)
  [Test] Add unit tests for HDR/TDigest PercentilesAggregators (elastic#24245)
  Fix FieldCaps documentation
  Upgrade to JUnit 4.12 (elastic#23877)
  Set available processors for Netty
  Painless: Fix method references to ctor with the new LambdaBootstrap and cleanup code (elastic#24406)
  Doc test: use propery regex for file size
  [DOCS] Tweak doc test to sync_flush
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants