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

Fix #222: initial implementation of native cron support #223

Merged
merged 3 commits into from
Jan 13, 2020

Conversation

nicolaferraro
Copy link
Member

Fix #222

This is a basic impl that needs some help to be improved.

I didn't go the source-loader way because source loaders cannot be easily mixed and this means technically e.g. that you cannot use the cron feature in a knative-source, because both require a distinct source loader (delegation is basic currently).

I've added two customizers, one for timer and one for quartz. Dependency to the respective camel libraries is optional, so loading a customizer does not take transitive dependencies of the other.

The customizer replaces options at endpoint level. This means that the Endpoint definition (that sometimes is printed in the logs) contains the original URI, but the endpoint values will be different at runtime. I don't know how much this is ok..

There's also a route policy that shuts the route down once the first exchange is completed (gracefully, to give time to all dependent exchanges to complete). This may not be sufficient in the Quarkus case as you mentioned (@davsclaus , @lburgazzoli ), because shutting down the context does not take down the application, right?

@lburgazzoli
Copy link
Contributor

Wonder if we should simply replace the input definition of the route with timer in any case as we don't need quartz overhead.

@nicolaferraro
Copy link
Member Author

Good point, but would you remove all quartz libraries on the operator side?

I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).

I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.

Both can work..

@lburgazzoli
Copy link
Contributor

lburgazzoli commented Jan 9, 2020

Good point, but would you remove all quartz libraries on the operator side?

Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer.

BTW, we should also try to determine if you have a single route and in such case we can apply the magic, otherwise we should just go for standard deployment.

I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).

Guess this can be controlled by a trait so the auto mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic

I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.

I think that if you want k8s scheduler, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will always be just 1)

@nicolaferraro
Copy link
Member Author

Good point, but would you remove all quartz libraries on the operator side?

Yes, I think in case we want to leverage k8s scheduler we can replace the discovered dependencies with what make more sense for us so in this case quartz --> timer

I did minimal variations to make sure an user can switch from kube-scheduled mode to standard mode easily (potentially even without changes in the library path).

Guess this can be controlled by a trait so the auto mode would do the magic but if you need fine grained control, then you can use the trait to influence the level of the magic

I've overridden the required fields, keeping other user settings on the endpoints.. even if I can't find something relevant that an user may customize on quartz. There are some headers added on the exchanges, that can be faked in case.

I think that if you want k8s jobs, then you do not care about headers generated by the camel components, if you want them, probably you need to keep your pod running as the headers won't be what you expect (i.e. the number of fired event will allays be just 1)

Yeah, I was referring to `quartz' that has a fire time... but that can be taken from system time.

Another benefit of the approach is that you can make it generic as it can replace all (ENV var) given endpoints with timers. And that literally allows you to plug new endpoints, directly from the operator.. even inventing new ones.. like kubernetes-cron:0/1 * * * *.

Let me do another attempt.. do you think Runtime.Listener is a good place to start?

@lburgazzoli
Copy link
Contributor

Thinking a little bit more, we can create a camel-cron component that like the camel-platform-http component can delegate to the a specific implementation

@nicolaferraro
Copy link
Member Author

Thinking a little bit more, we can create a camel-cron component that like the camel-platform-http component can delegate to the a specific implementation

That would be nice. A cron trait in the operator can choose the implementation. It will add quartz if needed, otherwise falling back on Kube. A reason to use quartz can be also having the route mixed with other active endpoints.

It should be able to support 5 to 7 items in the cron spec.

@lburgazzoli
Copy link
Contributor

sounds like a plan :) go ahead

@nicolaferraro
Copy link
Member Author

This second attempt uses a Runtime.Listener to override all specified components, no matter if they are in the classpath.

I'm opening an issue for the Camel cron component in the meantime.

@Override
public void onExchangeDone(Route route, Exchange exchange) {
LOG.info("Context shutdown started by cron policy");
context.getExecutorServiceManager().newThread("terminator", context::stop).start();
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need to add a new method stop() to the Runtime and invoke such method here instead of CamelContext::stop so the standard runtime would invoke Main::stop and the Quarkus one will invoke the correct method once provided by quarkus

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, sounds good!

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated it

@nicolaferraro nicolaferraro merged commit c703ea5 into apache:master Jan 13, 2020
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.

Native support for CronJob: convert cron endpoints to fire once at startup
3 participants