initial changes for yarn integration for dynamic scaling #1

Blazer-007 · 2024-11-13T15:22:21Z

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

My PR addresses the following Gobblin JIRA issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
- https://issues.apache.org/jira/browse/GOBBLIN-XXX

Description

Here are some details about my PR, including screenshots (if applicable):

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

…ulti-active DAG processing (apache#4074)

phet

good start!

...emporal/src/main/java/org/apache/gobblin/temporal/yarn/YarnServiceDynamicScalingManager.java

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/dynamic/WorkforcePlan.java

...emporal/src/main/java/org/apache/gobblin/temporal/yarn/YarnServiceDynamicScalingManager.java

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/YarnService.java

...emporal/src/main/java/org/apache/gobblin/temporal/yarn/YarnServiceDynamicScalingManager.java

…TPS host+port (apache#4075)

...emporal/src/main/java/org/apache/gobblin/temporal/yarn/DynamicScalingYarnServiceManager.java

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/DynamicScalingYarnService.java

phet · 2024-11-19T00:54:26Z

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/DynamicScalingYarnService.java

+    scalingDirectives.forEach(directive -> this.workforceStaffing.reviseStaffing(
+        directive.getProfileName(), directive.getSetPoint(), directive.getTimestampEpochMillis())
+    );


remember: some ScalingDirectives may be rejected by WorkforcePlan (e.g. all kinds of WorkforcePlan::IllegalRevisionException). accordingly, it's NOT safe to process them directly and make any presumptions about which to use. by contrast, it IS safe to iterate over StaffingDeltas

anyway, that may only be safer to perform inside requestNewContainersForStaffingDeltas, once each requestContainersForWorkerProfile succeeds

phet · 2024-11-19T00:59:52Z

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/DynamicScalingYarnService.java

+  private synchronized void requestContainersForWorkerProfile(WorkerProfile workerProfile, int numContainers) {
+    int containerMemoryMbs = workerProfile.getConfig().getInt(GobblinYarnConfigurationKeys.CONTAINER_MEMORY_MBS_KEY);
+    int containerCores = workerProfile.getConfig().getInt(GobblinYarnConfigurationKeys.CONTAINER_CORES_KEY);
+    long allocationRequestId = allocationRequestIdGenerator.getAndIncrement();


I'm unclear on how these IDs are to be used... but wanting to verify it's OK to request multiple containers together w/ the same ID vs. having a unique ID for each container requested. given we ultimately may request N containers together but later want to decomm only a subset, I'd have thought per-container IDs to be clearer.

edit: I see, you're using merely to look up the config for the container. for that, yes per-request ID looks fine. I was imagining for correlating to guide which to terminate for scale-down. for that, we'll probably need per-container IDs.

Yes for terminating or scaling down we will use container.getId() which is unique for every container.

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/DynamicScalingYarnService.java

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/YarnService.java

phet · 2024-11-19T02:34:27Z

gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/YarnService.java

should WorkforceStaffing adjustment occur as part of the AMRMClientCallbackHandler?

… with parser and `FsScalingDirectiveSource` (apache#4068)

…silon (apache#4076) where epsilon is the multi-active execution/lease consolidation period

…ve`s with parser

[GOBBLIN-2171] Streamline log messages for easier debugging of GaaS m…

13a6926

…ulti-active DAG processing (apache#4074)

phet reviewed Nov 13, 2024

View reviewed changes

[GOBBLIN-2172] Enable launching GoT YARN AM and workers with proxy HT…

4bb0c40

…TPS host+port (apache#4075)

phet reviewed Nov 19, 2024

View reviewed changes

phet and others added 9 commits November 19, 2024 07:49

[GOBBLIN-2170] Define Gobblin-on-Temporal dynamic ScalingDirectives…

4e38b2f

… with parser and `FsScalingDirectiveSource` (apache#4068)

[GOBBLIN-2173] Disallow adhoc flows sharing the same FlowId within ep…

e5d897e

…silon (apache#4076) where epsilon is the multi-active execution/lease consolidation period

Define Gobblin-on-TemporalWorkforcePlan and dynamic `ScalingDirecti…

8b8bb5a

…ve`s with parser

change package name from dynscale to dynamic and add a few comments

8eb3d4e

initial changes for yarn integration for dynamic scaling

a788a8f

added worker profile config to be used while starting container

f1c859c

removed getter

7e7113c

addressed few comments

0528f71

adding dummy scaling directive source for testing

8a51f6c

Blazer-007 force-pushed the dynamic-scaling-test-branch branch from 7273a0b to 8a51f6c Compare November 23, 2024 18:02

Blazer-007 mentioned this pull request Nov 23, 2024

[GOBBLIN-2174] GoT YarnService Integration with DynamicScaling apache/gobblin#4077

Open

Blazer-007 added 10 commits November 23, 2024 23:57

adding changes that was removed during rebase

64eb6e2

addressed few review comments

7094023

changed initial container request to use baseline worker profile

9ccf8ae

added abstractdynamicscalingmanger

606349e

fix typo and added one extra log line while launching initial containers

0c87a75

added unit test for buildcontainercommand

c7a6d1a

added unit test for baselineworkerprofilecreation

3883837

added unit tests for runnable

c478eb5

refactored tests

15396fc

minor comment update

e511968

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

initial changes for yarn integration for dynamic scaling #1

initial changes for yarn integration for dynamic scaling #1

Blazer-007 commented Nov 13, 2024

phet left a comment

phet Nov 19, 2024

phet Nov 19, 2024

Blazer-007 Nov 19, 2024

phet Nov 19, 2024

initial changes for yarn integration for dynamic scaling #1

Are you sure you want to change the base?

initial changes for yarn integration for dynamic scaling #1

Conversation

Blazer-007 commented Nov 13, 2024

JIRA

Description

Tests

Commits

phet left a comment

Choose a reason for hiding this comment

phet Nov 19, 2024

Choose a reason for hiding this comment

phet Nov 19, 2024

Choose a reason for hiding this comment

Blazer-007 Nov 19, 2024

Choose a reason for hiding this comment

phet Nov 19, 2024

Choose a reason for hiding this comment