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

CAMEL-12418: High CPU load on events watching #2283

Merged
merged 4 commits into from
Apr 18, 2018
Merged

CAMEL-12418: High CPU load on events watching #2283

merged 4 commits into from
Apr 18, 2018

Conversation

nryanov
Copy link
Contributor

@nryanov nryanov commented Apr 8, 2018

I believe that scheduledExecutorService with schedule method will be a little better then Thread.sleep(mills) (maybe i'm wrong).

Also, i didn't add default value as proposed in description of this problem because as i found in ConsulClientConfiguration the default value of blockSeconds is 10:

@UriParams
public class ConsulClientConfiguration implements Cloneable {
    . . . 
    @UriParam(label = "consumer,watch", defaultValue = "10")
    private Integer blockSeconds = 10;
    . . . 
}

Check style

Result of mvn clean install -Psourcecheck:

[INFO] Scanning for projects...
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building Camel :: Consul 2.22.0-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:3.0.0:clean (default-clean) @ camel-consul ---
[INFO] Deleting D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target
[INFO] 
[INFO] --- maven-enforcer-plugin:1.4.1:enforce (enforce-maven) @ camel-consul ---
[INFO] 
[INFO] --- maven-bundle-plugin:3.5.0:cleanVersions (versions) @ camel-consul ---
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.0.0:checkstyle (default-cli) @ camel-consul ---
[INFO] Starting audit...
Audit done.
[INFO] 
[INFO] --- maven-remote-resources-plugin:1.5:process (process-resource-bundles) @ camel-consul ---
[INFO] 
[INFO] --- camel-package-maven-plugin:2.22.0-SNAPSHOT:prepare-components (prepare) @ camel-consul ---
[INFO] Generated D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\generated\camel\components\META-INF\services\org\apache\camel\component.properties containing 1 Camel component: consul
[INFO] 
[INFO] --- maven-resources-plugin:3.0.2:resources (default-resources) @ camel-consul ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 4 resources
[INFO] Copying 3 resources
[INFO] Copying 1 resource
[INFO] skip non existing resourceDirectory D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\generated\camel\dataformats
[INFO] skip non existing resourceDirectory D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\generated\camel\languages
[INFO] 
[INFO] --- maven-compiler-plugin:3.7.0:compile (default-compile) @ camel-consul ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 37 source files to D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\classes
[INFO] 
[INFO] --- maven-resources-plugin:3.0.2:testResources (default-testResources) @ camel-consul ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 4 resources
[INFO] Copying 3 resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.7.0:testCompile (default-testCompile) @ camel-consul ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 23 source files to D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\test-classes
[INFO] 
[INFO] --- maven-surefire-plugin:2.20.1:test (default-test) @ camel-consul ---
[INFO] Tests are skipped.
[INFO] 
[INFO] --- maven-bundle-plugin:3.5.0:manifest (bundle-manifest) @ camel-consul ---
[WARNING] Manifest org.apache.camel:camel-consul:jar:2.22.0-SNAPSHOT : Unused Import-Package instructions: [org.springframework.ws.*, org.springframework.xml.*, org.springframework.*, org.apache.cxf.*, org.apache.qpid.*, org.apache.abdera.*, org.apache.commons.httpclient.*, org.apache.velocity.*, org.apache.xmlbeans.*, org.eclipse.jetty.*, com.thoughtworks.xstream.*, org.antlr.stringtemplate.*, org.ccil.cowan.tagsoup.*, org.mortbay.cometd.*, net.sf.flatpack.*, net.sf.saxon.*, freemarker.*, javax.persistence.*, org.apache.lucene.*, org.apache.solr.*] 
[INFO] 
[INFO] --- camel-package-maven-plugin:2.22.0-SNAPSHOT:validate-components (validate) @ camel-consul ---
[INFO] Validation complete
[INFO] 
[INFO] --- camel-package-maven-plugin:2.22.0-SNAPSHOT:prepare-spring-boot-starter (validate) @ camel-consul ---
[INFO] Spring-Boot-Starter: starter dir for the component is: D:\Program Files\WORK\WORK_Java\camel\platforms\spring-boot\components-starter\camel-consul-starter
[INFO] Reusing the existing pom.xml for the starter
[INFO] 
[INFO] --- camel-package-maven-plugin:2.22.0-SNAPSHOT:prepare-spring-boot-auto-configuration (validate) @ camel-consul ---
[INFO] Updated existing file: D:\Program Files\WORK\WORK_Java\camel\platforms\spring-boot\components-starter\camel-consul-starter\src\main\java\org\apache\camel\component\consul\springboot\ConsulComponentConfiguration.java
[INFO] Updated existing file: D:\Program Files\WORK\WORK_Java\camel\platforms\spring-boot\components-starter\camel-consul-starter\src\main\java\org\apache\camel\component\consul\springboot\ConsulComponentAutoConfiguration.java
[INFO] 
[INFO] --- maven-jar-plugin:3.0.2:jar (default-jar) @ camel-consul ---
[INFO] Building jar: D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\camel-consul-2.22.0-SNAPSHOT.jar
[INFO] 
[INFO] --- maven-site-plugin:3.5.1:attach-descriptor (attach-descriptor) @ camel-consul ---
[INFO] 
[INFO] --- camel-package-maven-plugin:2.22.0-SNAPSHOT:update-readme (readme) @ camel-consul ---
[INFO] 
[INFO] --- maven-install-plugin:2.5.2:install (default-install) @ camel-consul ---
[INFO] Installing D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\target\camel-consul-2.22.0-SNAPSHOT.jar to C:\Users\GrIfOn\.m2\repository\org\apache\camel\camel-consul\2.22.0-SNAPSHOT\camel-consul-2.22.0-SNAPSHOT.jar
[INFO] Installing D:\Program Files\WORK\WORK_Java\camel\components\camel-consul\pom.xml to C:\Users\GrIfOn\.m2\repository\org\apache\camel\camel-consul\2.22.0-SNAPSHOT\camel-consul-2.22.0-SNAPSHOT.pom
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 16.378 s
[INFO] Finished at: 2018-04-09T01:29:11+03:00
[INFO] Final Memory: 53M/553M
[INFO] ------------------------------------------------------------------------

Copy link
Contributor

@davsclaus davsclaus left a comment

Choose a reason for hiding this comment

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

We should have code that shutdown that executor service - also the executor service should be created via Camel's API for that, see ExecutorServiceManager you can get via CamelContext

@nryanov
Copy link
Contributor Author

nryanov commented Apr 10, 2018

Ok, i'll fix it

@davsclaus
Copy link
Contributor

@Gr1f0n6x let us know how it goes for you

@nryanov
Copy link
Contributor Author

nryanov commented Apr 17, 2018

@davsclaus, I tried to use ConsulEndpoint for getting CamelContext as it implemented CamelContextAware.
Also, i move field scheduledExecutorService to the ConsulEventConsumer class from the inner class EventWatcher because otherwise it will created more than one thread for this background work.
I'm not sure about doStop but i overrided it and shutdown executor there.

Sorry for delay. I had some problems with building the project

}

@Override
protected Runnable createWatcher(EventClient client) throws Exception {
return new EventWatcher(client);
}

@Override
protected void doStop() throws Exception {
scheduledExecutorService.shutdown();
Copy link
Member

Choose a reason for hiding this comment

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

@Gr1f0n6x , please use ExecutorServiceManager.shutdownGraceful there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmvolod, did it. Thank you for hint, didn't notice this method before


public ConsulEventConsumer(ConsulEndpoint endpoint, ConsulConfiguration configuration, Processor processor) {
super(endpoint, configuration, processor, Consul::eventClient);
this.executorServiceManager = endpoint.getCamelContext().getExecutorServiceManager();
this.scheduledExecutorService = this.executorServiceManager.newSingleThreadScheduledExecutor(this, "ConsulEventConsumer");
Copy link
Contributor

Choose a reason for hiding this comment

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

We create these in the doStart method, so we have start/stop logic together, and the constructors are light-weight

@davsclaus davsclaus merged commit 501045c into apache:master Apr 18, 2018
davsclaus pushed a commit that referenced this pull request Apr 18, 2018
* Add delay between listEvents requests

* Use ExecutorServiceManager got via CamelContext

* Add explicit field executorServiceManager

* Use ExecutorServiceManager.shutdownGraceful
@nryanov nryanov deleted the CAMEL-12418 branch April 18, 2018 12:21
tdiesler pushed a commit to tdiesler/camel that referenced this pull request Feb 20, 2019
* Add delay between listEvents requests

* Use ExecutorServiceManager got via CamelContext

* Add explicit field executorServiceManager

* Use ExecutorServiceManager.shutdownGraceful
tdiesler pushed a commit to tdiesler/camel that referenced this pull request Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants