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.

Sign up for GitHub

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

Jump to bottom

Safely register ClusterStateListener #38560

Open
7 of 26 tasks
pgomulka opened this issue Feb 7, 2019 · 2 comments
Open
7 of 26 tasks

Safely register ClusterStateListener #38560

pgomulka opened this issue Feb 7, 2019 · 2 comments
Assignees
@pgomulka
Labels
:Core/Infra/Core Core issues without another label help wanted adoptme Meta >refactoring Team:Core/Infra Meta label for core/infra team v8.15.0

Comments

@pgomulka
Copy link
Contributor

pgomulka commented Feb 7, 2019 •
edited
Loading

This is a follow up from #37861 discussed on core-infra sync meeting on 6th of Feb19 . See the original issue for reasoning behind this.

The goal is to find usages of ClusterStateListener and make sure it is not publishing its this reference during construction. There might be multiple possible ways to fix that.
The easiest fix is to just extract a factory method. However when there is multiple overloaded constructors, it might be tempted to remove the ClusterService parameter and register in a place where the instance is created.
In some cases where constructor parameters are provided with @Inject the registration could be done in AbstractLifecycleComponent.start/doStart.
Or refactored to factory method with @Inject (that annotation might work with methods, although couldn't find usages.

  • DelayedAllocationService (org.elasticsearch.cluster.routing)
  • MlInitializationService (org.elasticsearch.xpack.ml)
  • LocalNodeMasterListeners in ClusterApplierService (org.elasticsearch.cluster.service)
  • LicenseService (org.elasticsearch.license)
  • WatcherIndexTemplateRegistry (org.elasticsearch.xpack.watcher.support)
  • DanglingIndicesState (org.elasticsearch.gateway)
  • WatcherIndexingListener (org.elasticsearch.xpack.watcher)
  • AutoFollowCoordinator (org.elasticsearch.xpack.ccr.action)
  • IndicesStore (org.elasticsearch.indices.store)
  • InternalClusterInfoService (org.elasticsearch.cluster)
  • WatcherLifeCycleService (org.elasticsearch.xpack.watcher)
  • ResponseCollectorService (org.elasticsearch.node)
  • SecurityIndexManager (org.elasticsearch.xpack.security.support)
  • PersistentTasksClusterService (org.elasticsearch.persistent)
  • MlAssignmentNotifier (org.elasticsearch.xpack.ml)
  • AutodetectProcessManager (org.elasticsearch.xpack.ml.job.process.autodetect)
  • SnapshotShardsService (org.elasticsearch.snapshots)
  • IndexLifecycleService (org.elasticsearch.xpack.indexlifecycle)
  • PersistentTasksNodeService (org.elasticsearch.persistent)
  • TaskRunner in DatafeedManager (org.elasticsearch.xpack.ml.datafeed) (inner class is indirectly also publishing the this of outter class)
  • LoggingAuditTrail (org.elasticsearch.xpack.security.audit.logfile)
  • TemplateUpgradeService (org.elasticsearch.cluster.metadata)
  • GatewayService (org.elasticsearch.gateway) sample of how to register in AbstractLifecycleComponent.doStart
  • LocalExporter (org.elasticsearch.xpack.monitoring.exporter.local)
  • RestoreClusterStateListener (org.elasticsearch.action.admin.cluster.snapshots.restore) registered in a separate method (not a static factory method) - another way of approaching this
  • GeoIpDownloaderTaskExecutor
@pgomulka pgomulka added :Core/Infra/Core Core issues without another label Meta >refactoring v8.0.0 labels Feb 7, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@pgomulka pgomulka self-assigned this Feb 7, 2019
@pgomulka pgomulka mentioned this issue Feb 7, 2019
Closed
@jasontedor jasontedor mentioned this issue Mar 18, 2019
Merged
talevy added a commit to talevy/elasticsearch that referenced this issue Aug 13, 2019
@talevy
Safe publication of DelayedAllocationService and SnapshotShardsService
a0dd1aa 
These two classes were leaking a reference to themselves when adding themselves
as listeners to the cluster service prior to being completely initialized

these two instances were fixed by leveraging doStart/doStop to represent
start/stop lifecycle points for when these objects are registered listeners.

Relates elastic#38560
@talevy talevy mentioned this issue Aug 13, 2019
Closed
@jaymode jaymode mentioned this issue Jan 29, 2020
Closed
@jaymode jaymode mentioned this issue Mar 5, 2020
Open
@droberts195 droberts195 mentioned this issue Apr 28, 2020
Merged
@rjernst rjernst added the Team:Core/Infra Meta label for core/infra team label May 4, 2020
@pgomulka pgomulka mentioned this issue Jul 15, 2020
Closed
@dakrone dakrone mentioned this issue Oct 15, 2020
Merged
@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@gwbrown gwbrown added help wanted adoptme and removed needs:triage Requires assignment of a team area label labels Dec 14, 2020
@arteam arteam added v8.1.0 and removed v8.0.0 labels Jan 12, 2022
@mark-vieira mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022
@mark-vieira mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@csoulios csoulios added v8.6.0 and removed v8.5.0 labels Sep 21, 2022
@kingherc kingherc added v8.7.0 and removed v8.6.0 labels Nov 16, 2022
@rjernst rjernst added v8.8.0 and removed v8.7.0 labels Feb 8, 2023
@gmarouli gmarouli added v8.9.0 and removed v8.8.0 labels Apr 26, 2023
@quux00 quux00 added v8.11.0 and removed v8.10.0 labels Aug 16, 2023
@mattc58 mattc58 added v8.12.0 and removed v8.11.0 labels Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pgomulka pgomulka

Labels
:Core/Infra/Core Core issues without another label help wanted adoptme Meta >refactoring Team:Core/Infra Meta label for core/infra team v8.15.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests