Initial commit to split timers to own process #4180

lakshmi-kannan · 2018-06-15T17:15:07Z

What?

This PR splits out the timer portion (one that injects trigger instances based on user specified cron expressions in rules) into its own process.

Why?

For the kubernetes (k8s) HA story, we are going to rely on a single timers engine container with failover handed natively by k8s. The scale requirements for timers aren't that rigorous and the timer doesn't do anything other than inject a trigger instance into rabbitmq. This makes the story for timers simple. This also allows us to scale rules engine horizontally without worrying about partitioning timers. Since rules engine don't modify state (other than add operational entries to DB), we can use k8s to set a scale number for rules engine and handle both failover and scaling with k8s primitives. So we make scaling rules engine story simpler too.

In the future, we can decide to split timers into different partitions and go for a more complex HA model if needed. So this change will enable future scaling optimizations for timers. We are also thinking about adding the scheduling logic into this for workflow orchestrators.

TODO

Packaging changes Add init files for all OSes for st2timersengine [ci skip] st2-packages#564
Production log configurations
st2ctl to start/stop
Documentation (No special documentation needed)
Upgrade notes Upgrade notes for new st2timersengine st2docs#749
CI upgrade testing (Install 2.7.2, enable timer rule, upgrade to latest unstable, see if rule fires still)
Changelog
st2docs change

arm4b · 2018-06-15T18:56:02Z

conf/st2.conf.sample

 enable = True
 # Timezone pertaining to the location where st2 is run.
-local_timezone = America/Los_Angeles
+local_tz = America/Los_Angeles
+logging = st2reactor/conf/logging.timersengine.conf


Probably logging = conf/logging.timersengine.conf fits this config

cognifloyd · 2018-06-15T19:42:37Z

conf/st2.conf.sample

 enable = True
 # Timezone pertaining to the location where st2 is run.
-local_timezone = America/Los_Angeles
+local_tz = America/Los_Angeles


local_tz vs local_timezone ?

It was auto-generated but I'll look into why the auto-gen code did this.

arm4b · 2018-07-18T12:43:04Z

@lakshmi-kannan Since we're on 2.9 roadmap now, is it possible to complete this PR so we'll start integrating with the new st2timersegine?

An additional request to include a HA description for the service in
https://docs.stackstorm.com/reference/ha.html#components

* master: (235 commits) Use default scope of "all" for list command and "system" for get, set and delete commands. Update ALLOWED_SCOPES - all should not be there. Make http runner password parameter a secret. Update CHANGELOG.rst Use consistent formatting. Sync changelog with v2.8.1 release. Fix typo in description Remove unused variable. Replace get_terminal_size with get_terminal_size_columns. Number the various fallbacks. Add tests for get_terminal_size. Add a note. Update get_terminal_size method to check LINES and COLUMNS environment variables first. Rewording. Add changelog entry. Truncate extra whitespace. Make sure we cast it to int. 200 -> 150.. Also use a more reasonable default terminal size. Allow user to force terminal size used by the st2 CLI formattes. ...

* master: Add a test case for it. Simplify the logic, fix test which didn't pass in Content-Type header. Update .gitignore. Also blacklist webhooks API endpoint which can take multipart/form-data content type. Add a workaround for eventlet WSGI http server. Refactor orchestra conductor interface to support the state machine updates

Kami · 2018-07-23T15:45:23Z

st2reactor/st2reactor/cmd/timersengine.py

+            LOG.info(TIMER_ENABLED_LOG_LINE)
+            return timer_thread.wait()
+        else:
+            LOG.info(TIMER_DISABLED_LOG_LINE)


It looks like if timer engine is disabled service will just exit immediately on startup, right?

Just something to keep in mind / document for monitoring purposes (e.g. if timer engine service is disables, st2timerengine service won't be running and it will exit immediately on startup).

Correct. +1 to documenting it in monitoring docs.

Documented: StackStorm/st2docs@30e714a

Kami · 2018-07-23T15:45:48Z

st2reactor/st2reactor/cmd/timersengine.py

+
+    try:
+        timer_thread = None
+        if cfg.CONF.timer.enable or cfg.CONF.timersengine.enable:


It would be a bit config is one is False and other is True, so perhaps we should be more explicit and throw in such scenario? Or?

With default configs, you'll always have this behavior. So we have to actually detect if those variables are defined in /etc/st2/st2.conf. I think that's not really required. We have documented in upgrade notes. And when people upgrade, the configuration change diff will be shown to them too. So there are multiple checks and alerts to the user.

One thing which is also config is that because we default one option to True, when user wants to disable the service they need to set both options to False.

I will document that (if not already).

Kami · 2018-07-23T15:46:32Z

Two small comments, besides that, LGTM.

* master: (172 commits) Remove global cache_name environment variable definition. Ignore CryptographyDeprecationWarning deprecation warning which appears on our Ubuntu build server which runs old 2.7 release. Don't run tests under MongoDB 3.6 until we figure out why the tests are so much slower under 3.6. Add new examples.python_runner_print_python_environment action which will allow us to debug various Python runner action issues. Instead of failing the build, just warn if the job exceeds the thresold. Make sure mongodb user can write to the lib dir. Make sure we clean any old MongoDB 3.4 files laying around otherwise the service won't start. Only tail last 30 lines. Cat mongo log to see what is going on. Remove lines we don't need. Check service status. Use longer sleep. Use longer thresholds. Fix syntax error. Also print out mongod version. Add a new Travis build task which runs tests under MongoDB 3.6. MongoDB 3.6 supports 64 bit ints, update affected tests. Add changelog entry. Also upgrade pymongo. Upgrade to our forked version of mongoengine which is based on v0.15.3 and contains a fix for regression in memory usage introduced in v0.13.0. ...

Kami · 2018-08-07T10:47:44Z

I will take over that and try to get it finished and merged this week.

timersengine config section.

Kami · 2018-08-07T13:09:34Z

Pushed some fixes and test changes - 2422b8c, 04d0d98, 21dea06.

As mentioned above, I'm still not a fan of this duplicated enable option which makes for a confusing behavior, but it is what is is now.

Kami · 2018-08-07T13:54:45Z

There is a chicken and the egg problem with our e2e tests - they depend on st2-packages changes.

But for that changes to work, this PR needs to be merged first.

Lakshmi Kannan added 4 commits June 15, 2018 13:05

Initial commit to split timers to own process

9390a4c

Fix st2.conf.sample

e665dbd

Fix missing imports

d19aca3

Remove duplicate config registration

b87cfe7

arm4b reviewed Jun 15, 2018

View reviewed changes

cognifloyd reviewed Jun 15, 2018

View reviewed changes

Lakshmi Kannan added 2 commits June 18, 2018 11:49

Fix entries in st2.conf.sample [ci skip]

7a3e8e3

Fix config sample auto-gen

0a6701d

arm4b added the K8s label Jun 18, 2018

Lakshmi Kannan added 3 commits June 19, 2018 10:58

Fix st2ctl to include st2timersengine component [ci skip]

4b9a3dd

Production logging changes for st2timersengine component [ci skip]

2c4e919

Path to logging file is /etc/st2/logging.timersengine.conf [ci skip]

6dcfff2

lakshmi-kannan added this to the 2.9.0 milestone Jun 19, 2018

lakshmi-kannan changed the title ~~WIP: Initial commit to split timers to own process~~ DO NOT MERGE UNTIL 2.8 RELEASE: Initial commit to split timers to own process Jun 19, 2018

lakshmi-kannan mentioned this pull request Jun 20, 2018

Upgrade notes for new st2timersengine StackStorm/st2docs#749

Merged

arm4b mentioned this pull request Jul 18, 2018

Document st2timersengine in HA StackStorm/st2docs#766

Closed

lakshmi-kannan changed the title ~~DO NOT MERGE UNTIL 2.8 RELEASE: Initial commit to split timers to own process~~ Initial commit to split timers to own process Jul 19, 2018

Lakshmi Kannan added 2 commits July 19, 2018 15:24

Kami reviewed Jul 23, 2018

View reviewed changes

lakshmi-kannan and others added 2 commits July 24, 2018 10:31

Merge branch 'master' into k8s/split_timers

4b5b7a6

Kami approved these changes Aug 6, 2018

View reviewed changes

Kami added 3 commits August 7, 2018 14:57

Update affected tests.

cb660e1

Fix function names.

2422b8c

Fix a bug with "enable" config option not being registered under

04d0d98

timersengine config section.

Update affected tests.

21dea06

Re-generate sample config.

d1fd899

This was referenced Aug 7, 2018

Add init files for all OSes for st2timersengine [ci skip] StackStorm/st2-packages#564

Merged

Remove ST2_GITREV - this change is now in st2 master StackStorm/st2-packages#581

Merged

Kami merged commit 946060c into master Aug 7, 2018

Kami deleted the k8s/split_timers branch August 7, 2018 14:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial commit to split timers to own process #4180

Initial commit to split timers to own process #4180

lakshmi-kannan commented Jun 15, 2018 •

edited

Loading

arm4b Jun 15, 2018 •

edited

Loading

cognifloyd Jun 15, 2018

lakshmi-kannan Jun 18, 2018

arm4b commented Jul 18, 2018 •

edited

Loading

Kami Jul 23, 2018

lakshmi-kannan Jul 24, 2018

lakshmi-kannan Jul 24, 2018

Kami Jul 23, 2018

lakshmi-kannan Jul 24, 2018

Kami Aug 7, 2018

Kami commented Jul 23, 2018

Kami commented Aug 7, 2018

Kami commented Aug 7, 2018

Kami commented Aug 7, 2018

Initial commit to split timers to own process #4180

Initial commit to split timers to own process #4180

Conversation

lakshmi-kannan commented Jun 15, 2018 • edited Loading

What?

Why?

TODO

arm4b Jun 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b commented Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kami commented Jul 23, 2018

Kami commented Aug 7, 2018

Kami commented Aug 7, 2018

Kami commented Aug 7, 2018

lakshmi-kannan commented Jun 15, 2018 •

edited

Loading

arm4b Jun 15, 2018 •

edited

Loading

arm4b commented Jul 18, 2018 •

edited

Loading