The zoo is under new management (zk cleanup) by ssalinas · Pull Request #1922 · HubSpot/Singularity

ssalinas · 2019-04-22T13:24:27Z

Be much nicer to zookeeper by avoiding possible large payloads, specifically:

Namespace pending tasks by request id so that the list children call for this does not get to large when there are thousands of pending tasks
- Avoid the list + filter call, which required fetching all pending task ids to find the ones for a particular request
Remove usage of the /tasks/active node entirely since we have this data elsewhere
Namespaces last active task statuses nodes by request id and use this data to power the active tasks list instead
clean up nodes from old features that are no longer used

Still TODO on this PR:

automatic cleanup of request groups when a request is deleted
revisit caching to determine if any additional data can be leader cached
more aggressive pruning of inactive slaves data

…nfig.

…nding tasks

ssalinas · 2019-04-22T15:17:17Z

We weren't using the correct code paths to hit the leader cache for slave + rack data, causing lots of unneeded zk calls for those. Fixed in this PR now

ssalinas · 2019-04-22T17:52:07Z

/fixes #1823

baconmania · 2019-04-25T07:29:02Z

+  public Optional<SingularityRack> getObjectFromLeaderCache(String rackId) {
    if (leaderCache.active()) {
-      return leaderCache.getRack(rackName);
+      return leaderCache.getRack(rackId);


We're using both rackName (in SingularityLeaderCache) and rackId. We should pick one for consistency

consolidating on rackId 👍

baconmania · 2019-04-25T07:30:01Z

+        List<String> currentActive = curatorFramework.getChildren().forPath(ACTIVE_STATUSES_ROOT);
+        for (String taskIdString : currentActive) {
+          SingularityTaskId taskId = SingularityTaskId.valueOf(taskIdString);
+          String oldPath = ZKPaths.makePath(ACTIVE_STATUSES_ROOT, taskIdString);


We're running this code based on whether ACTIVE_TASKS_ROOT exists in ZK, but we're never actually reading from that path during this migration. Just double-checking, is that intended?

We remove it as the last step of the migration which is why it's my initial check. There isn't actually any data in those nodes, they are only an index we call getChildren on. The /tasks/statuses is where the actual data is and what I am namespacing. However, realizing that if this died half way though, some of the children on the next run would be request ids instead. Going to add a fix so this will be able to run on a half-migrated dataset as well

baconmania · 2019-04-25T07:30:22Z

      }

+      // Should be called before activation of leader cache or cache could be left empty
+      startup.checkMigrations();


Do we expect that this wouldn't have run at startup?

It only runs when we take leadership, not as a Managed class. Moved it here since we need to migrate before filling leader cache, since leader cache is using the newer taskManager at this point

baconmania · 2019-04-25T07:30:34Z

-    }
-
-    saveResult(Optional.of(response.getStatusCode()), responseBody, Optional.<String> absent(), Optional.<Throwable>absent());
+  public void setHealthcehckUri(String healthcheckUri) {


nit: spelling

baconmania · 2019-04-25T07:31:06Z

    <dep.classmate.version>1.3.1</dep.classmate.version>
    <dep.commons-lang3.version>3.7</dep.commons-lang3.version>
-    <dep.curator.version>2.12.0</dep.curator.version>
+    <dep.curator.version>4.2.0</dep.curator.version>


Sweet glory

baconmania · 2019-04-25T07:35:34Z


+  public void clearOldHistory(String machineId) {
+    List<SingularityMachineStateHistoryUpdate> histories = getHistory(machineId);
+    histories.sort(Comparator.comparingLong(SingularityMachineStateHistoryUpdate::getTimestamp));


I think we want to reverse this sort, or the skip() below will keep the oldest n entries instead of the newest.

Reversing the sort. Though I'm realizing we don't even really expose this data anywhere... Maybe don't even need it

baconmania · 2019-04-25T13:32:26Z

🚢

baconmania and others added 29 commits April 26, 2018 14:52

Pull in HTTP2-capable client.

1566cfa

Add & document new protocol options.

b17afad

Bind client.

9ab089a

Don't directly implement Ning-specific handler.

2c55771

Wrap handler & perform healthcheck using appropriate client.

cb64ac2

Manage this dependency here until it's moved to basepom.

8435dda

Add managed client which closes underlying resources at shutdown.

6062442

Bind managed client.

4de7bbe

Configure client before calling healthcheck.

9e05566

Add enum for supported healthcheck methods.

2e96899

Wire up healthcheck method in options object.

8c5f0d0

Add new healthcheck parameter to builder.

260df36

Respect configured healthcheck method.

da22938

Satisfy this constructor via deprecated deploy healthcheck options co…

2a54acb

…nfig.

Update equals()

9b15faf

Merge branch 'master' into http2-healthchecks

dd9f98c

Re-sync with master.

54f100c

UI support.

f8ae2e3

Make Singularity report byte counts to monitor against jute buffer size

ba3cecd

migration to clean old zk nodes, namespace active tasks, namespace pe…

d9c4dbf

…nding tasks

test fixing, test migration

d6d4b8e

do this at the end

f671687

bump dep versions

545224d

fix ning async update

d5a1dac

more test fixing form ning update

255de95

auto-clean request groups

0bb47e7

also clean existing

9465b08

cleaner

f29b402

actually leader cache slave and rack data, and actually use it

d84badd

ssalinas added the hs_staging label Apr 22, 2019

ssalinas changed the title ~~The zoo is under new management~~ The zoo is under new management (zk cleanup) Apr 22, 2019

ssalinas added 2 commits April 22, 2019 13:50

clear old slave history entries as well

fa432ca

Merge branch 'zk_bytes_metrics' into general_zk_cleanup

0b2db9c

remove task id bytes calc

123a669

ssalinas mentioned this pull request Apr 22, 2019

Make Singularity report byte counts to monitor against jute buffer size #1915

Closed

ssalinas added 5 commits April 22, 2019 14:59

bump zk back down to 3.4.x

fe0565e

older curator-test version

fc2e60a

fix pending path parent calc

1ef26f9

check zk migrations before filling leader cache

3148ed9

turn down test log level

0131f00

baconmania reviewed Apr 22, 2019

View reviewed changes

Comment thread SingularityService/src/main/java/com/hubspot/singularity/data/AbstractMachineManager.java

better fallback to zk from leader cache

8c9ae8e

ssalinas added the hs_qa label Apr 23, 2019

baconmania reviewed Apr 25, 2019

View reviewed changes

fix for PR comments

e7231c7

ssalinas added hs_stable and removed hs_stable labels Apr 25, 2019

ssalinas merged commit aeab026 into master Apr 25, 2019

ssalinas deleted the general_zk_cleanup branch April 25, 2019 20:19

ssalinas added this to the 0.23.0 milestone Jun 7, 2019

Conversation

ssalinas commented Apr 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ssalinas commented Apr 22, 2019

Uh oh!

ssalinas commented Apr 22, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baconmania Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baconmania commented Apr 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ssalinas commented Apr 22, 2019 •

edited

Loading

baconmania Apr 25, 2019 •

edited

Loading