Added mechanism to rampup Containerized from POLL dispatch #2779

aditya1105 · 2021-02-26T20:27:06Z

This ramp-up mechanism requires that containerization be enabled and ramp-up percentages are set appropriately. Setting it to 100 will make all the executions use Containerization and setting it to 0 implies they will be polled by the executor server.

This allows ContainerizedDispatchManager to submit/upload some flows to the execution_flows table based on the ramp percentage with an additional column value dispatch_method. Whenever executions are pulled from the execution_flows table, respective dispatch managers will query by filtering the dispatch_method. Any future changes to the ContainerizedDispatchManager will automatically be honored as there is no new DispatchManager.

If this mechanism needs to removed then the only change will be required in the class azkaban.executor.container.ContainerizedDispatchManager where getDispatchMethod will be changed to always return DispatchMethod.CONTAINERIZED

Following is the summary of the change:

Added a new column dispatch_method in the table exectuion_flows
Modified the getDispatchMethod() in the azkaban.executor.container.ContainerizedDispatchManager to determine the dispatch based on the ramp-up value set in the constructor.
getDispatchMethod() is used while the flow is submitted by azkaban.executor.AbstractExecutorManagerAdapter#uploadExecutableFlow
Modified the azkaban.executor.ExecutionFlowDao to make sure that dispatch_method value is also added to the execution_flows table while invoking uploadExecutableFlow()
Modified the azkaban.executor.ExecutionFlowDao to make sure that all selectAndUpdateExecution methods will require DispatchMethod as a parameter for querying.
Whenever selectAndUpdateExecution methods are used respective DispatchManagers will pass the appropriate DispatchMethod and hence only the corresponding executions are polled.

codecov · 2021-02-26T20:35:13Z

Codecov Report

Merging #2779 (7baff60) into master (270defa) will decrease coverage by 0.00%.
The diff coverage is 65.00%.

@@             Coverage Diff              @@
##             master    #2779      +/-   ##
============================================
- Coverage     39.73%   39.72%   -0.01%     
- Complexity     4159     4171      +12     
============================================
  Files           569      569              
  Lines         37508    37544      +36     
  Branches       4365     4370       +5     
============================================
+ Hits          14903    14914      +11     
- Misses        21359    21382      +23     
- Partials       1246     1248       +2

Impacted Files	Coverage Δ	Complexity Δ
az-core/src/main/java/azkaban/Constants.java	`23.07% <ø> (ø)`	`1.00 <0.00> (ø)`
...ain/java/azkaban/executor/ExecutionController.java	`37.20% <ø> (ø)`	`10.00 <0.00> (ø)`
...main/java/azkaban/executor/JdbcExecutorLoader.java	`16.85% <0.00%> (ø)`	`3.00 <0.00> (ø)`
...c/main/java/azkaban/execapp/FlowRunnerManager.java	`14.46% <0.00%> (ø)`	`7.00 <0.00> (ø)`
...ecutor/container/ContainerizedDispatchManager.java	`53.95% <41.17%> (-2.15%)`	`15.00 <3.00> (+2.00)`	⬇️
...n-common/src/main/java/azkaban/DispatchMethod.java	`94.11% <100.00%> (+1.80%)`	`8.00 <2.00> (+2.00)`
...kaban/executor/AbstractExecutorManagerAdapter.java	`54.43% <100.00%> (+0.18%)`	`28.00 <0.00> (ø)`
...src/main/java/azkaban/executor/ExecutableFlow.java	`89.75% <100.00%> (+0.18%)`	`50.00 <2.00> (+2.00)`
...c/main/java/azkaban/executor/ExecutionFlowDao.java	`62.26% <100.00%> (+0.35%)`	`29.00 <0.00> (ø)`
...azkaban/execapp/event/JobCallbackRequestMaker.java	`48.95% <0.00%> (-18.76%)`	`6.00% <0.00%> (ø%)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 270defa...7baff60. Read the comment docs.

coveralls · 2021-02-26T20:36:39Z

Pull Request Test Coverage Report for Build 5965

28 of 40 (70.0%) changed or added relevant lines in 7 files are covered.
21 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.04%) to 43.184%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
azkaban-common/src/main/java/azkaban/executor/JdbcExecutorLoader.java	0	2	0.0%
azkaban-exec-server/src/main/java/azkaban/execapp/FlowRunnerManager.java	0	2	0.0%
azkaban-common/src/main/java/azkaban/executor/container/ContainerizedDispatchManager.java	9	17	52.94%

Files with Coverage Reduction	New Missed Lines	%
azkaban-exec-server/src/main/java/azkaban/execapp/event/JobCallbackRequestMaker.java	21	56.25%

Totals
Change from base Build 5964:	-0.04%
Covered Lines:	16147
Relevant Lines:	37391

💛 - Coveralls

sshardool

Thanks a lot for looking into this.
As part of containerized dispatch we mode modifications to both ExecutorApiGateway and ExecutorApiClient. Most of them related to the reverse proxy but fairly orthogonal - including, TLS client support, generating prefix and making calls agains the reverse-proxy (ambassador) uri.
For executing any API calls against both the bare-metal executors as well as the FlowContainers we will likely need to have two separate instances of the ExecutorApiGateway with different configs.
Wanted to confirm our plan for addressing it.

abhisheknath2011 · 2021-03-02T10:08:49Z

azkaban-common/src/main/java/azkaban/executor/container/ContainerizedDispatchManager.java

@@ -123,7 +138,18 @@ public long getQueuedFlowSize() {
   */
  @Override
  public DispatchMethod getDispatchMethod() {
-    return DispatchMethod.CONTAINERIZED;
+    if (this.rampUp == 0) {


DispatchMethod is very useful and it controlling the behaviour at the moment by turning off and turning on feature. DispatchMethod is currently specified through configs. Hence, dispatch method is initialized during web server startup. The containerization feature is turned off if dispath method is POLL. If dispatch method is set to CONTAINERIZED then only all the guava juice binding for containerization are initialized. As we are dynamically toggling between POLL and CONTAINERIZED based on rampup we need to reload the bindings as well. In other words, we need to always intialized the bindings or look at different way of handling this.

+1. I didn't see any changes related to injection over here. How will implementation for POLL method be injected in case of ramp up?

There is a difference between DispatchMethod.CONTAINERIZED specified through the configs and that will always be DispatchMethod.CONTAINERIZED for the ramp-up feature.
Here it is about the return value of getDispatchMethod() which till now is not used anywhere in the code and hence doesn't affect anything. The return value of this is utilized to set the value of dispatch_method column in the execution_flows table while inserting the entry to the table. Hence at no place injections will be affected.

Got it. And specifying dispatch method as POLL from executor side will take care of polling executions. Thank you for clarifying this.

jakhani · 2021-03-08T21:37:57Z

azkaban-common/src/main/java/azkaban/DispatchMethod.java

+    this.numVal = numVal;
+  }
+
+  public int getNumVal() {


Is there any need to add method which will return enum for given num value?
What is the significance of this num value? Can you please add it in comment?

Till now we don't have a use-case for num to enum.
We are using Int as the data-type of dispatch_method in the execution_flows table and hence getNumVal is needed.

jakhani · 2021-03-08T21:48:18Z

azkaban-common/src/main/java/azkaban/executor/ExecutionFlowDao.java

@@ -84,7 +87,8 @@ public void uploadExecutableFlow(final ExecutableFlow flow)
    final SQLTransaction<Long> insertAndGetLastID = transOperator -> {
      transOperator.update(INSERT_EXECUTABLE_FLOW, flow.getProjectId(),
          flow.getFlowId(), flow.getVersion(), flow.getStatus().getNumVal(),
-          submitTime, flow.getSubmitUser(), submitTime, executorId, flowPriority, executionSource);
+          submitTime, flow.getSubmitUser(), submitTime, executorId, flowPriority, executionSource
+          , dispatchMethod.getNumVal());


I assume you are keeping num value for the optimization that int will provide from database side instead of enum name itself.

jakhani · 2021-03-08T21:52:45Z

azkaban-common/src/main/java/azkaban/executor/container/ContainerizedDispatchManager.java

@@ -123,7 +138,18 @@ public long getQueuedFlowSize() {
   */
  @Override
  public DispatchMethod getDispatchMethod() {
-    return DispatchMethod.CONTAINERIZED;
+    if (this.rampUp == 0) {


+1. I didn't see any changes related to injection over here. How will implementation for POLL method be injected in case of ramp up?

jakhani · 2021-03-08T21:56:49Z

azkaban-db/src/main/sql/create.containerization-tables-all.sql

+
+-- TODO: Add the alter table script in the specific release
+-- Adding dispatch_method column in execution_flows
+-- alter table execution_flows add column dispatch_method TINYINT default 1;


Can you please add index for this field? It is in the where clause in select query so it will be useful to have it.

jakhani · 2021-03-08T21:57:02Z

azkaban-db/src/main/sql/create.execution_flows.sql

@@ -15,6 +15,7 @@ CREATE TABLE execution_flows (
  use_executor INT                  DEFAULT NULL,
  flow_priority TINYINT    NOT NULL DEFAULT 5,
  execution_source VARCHAR(32)        DEFAULT NULL,
+  dispatch_method TINYINT        DEFAULT 1,


Can you please add index for this field?

Upgrade sql was not added for this change, but it should be (under https://github.com/azkaban/azkaban/tree/master/azkaban-db/src/main/sql).

Same for the added CREATE INDEX below.

Edit: fix candidate here #2932

jakhani

Thank you for taking care of all the comments.

sshardool reviewed Mar 1, 2021

View reviewed changes

abhisheknath2011 reviewed Mar 2, 2021

View reviewed changes

jakhani reviewed Mar 8, 2021

View reviewed changes

Added mechanism to rampup Containerized from POLL dispatch

7baff60

aditya1105 force-pushed the master branch from c3060d7 to 7baff60 Compare March 10, 2021 12:09

jakhani approved these changes Mar 10, 2021

View reviewed changes

aditya1105 merged commit 2ab81c1 into azkaban:master Mar 10, 2021

aditya1105 mentioned this pull request Mar 10, 2021

ApiGateway support for ramp-up mechanism from POLL to CONTANERIZED dispatch #2785

Merged

juhoautio mentioned this pull request Aug 31, 2021

Fix DB upgrades of execution_flows #2932

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added mechanism to rampup Containerized from POLL dispatch #2779

Added mechanism to rampup Containerized from POLL dispatch #2779

aditya1105 commented Feb 26, 2021

codecov bot commented Feb 26, 2021 •

edited

coveralls commented Feb 26, 2021 •

edited

sshardool left a comment

abhisheknath2011 Mar 2, 2021

jakhani Mar 8, 2021

aditya1105 Mar 9, 2021

jakhani Mar 10, 2021

jakhani Mar 8, 2021

aditya1105 Mar 9, 2021

jakhani Mar 8, 2021

aditya1105 Mar 9, 2021

jakhani Mar 8, 2021

jakhani Mar 8, 2021

aditya1105 Mar 10, 2021

jakhani Mar 8, 2021

aditya1105 Mar 10, 2021

juhoautio-rovio Aug 31, 2021 •

edited

jakhani left a comment

Added mechanism to rampup Containerized from POLL dispatch #2779

Added mechanism to rampup Containerized from POLL dispatch #2779

Conversation

aditya1105 commented Feb 26, 2021

codecov bot commented Feb 26, 2021 • edited

Codecov Report

coveralls commented Feb 26, 2021 • edited

Pull Request Test Coverage Report for Build 5965

💛 - Coveralls

sshardool left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juhoautio-rovio Aug 31, 2021 • edited

Choose a reason for hiding this comment

jakhani left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 26, 2021 •

edited

coveralls commented Feb 26, 2021 •

edited

juhoautio-rovio Aug 31, 2021 •

edited