Add processors to autoscaling capacity response #87895

benwtrent · 2022-06-21T16:31:25Z

This adds processors to the autocaling capacity response.

processors in the current capacity calculation is determined by the allocated_processors field in OsInfo. The reason behind this is because allocated_processors can be overridden by the process starting the node. Consequently, when autoscaling deciders need to look at the current processors and request for more, they should look at allocated_processors not available_processors. Mostly, this is because certain systems may want to account for threads being used by external processes and not allow ES to use all the available processors on the OS.

elasticmachine · 2022-06-21T16:31:29Z

Pinging @elastic/es-distributed (Team:Distributed)

elasticsearchmachine · 2022-06-21T16:31:51Z

Hi @benwtrent, I've created a changelog YAML for you.

benwtrent · 2022-06-21T18:44:48Z

Verified that response values are correct given the current capacities for various tiers. Responses containing current capacity look like:

 "current_capacity": {
        "node": {
          "storage": 128849018880,
          "memory": 4294967296,
          "processors": 2
        },
        "total": {
          "storage": 257698037760,
          "memory": 8589934592,
          "processors": 4
        }
      }

This matches up with true node values based on OsInfo.

droberts195

LGTM

droberts195 · 2022-06-22T09:20:04Z

...oscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCapacity.java


-        public static final AutoscalingResources ZERO = new AutoscalingResources(new ByteSizeValue(0), new ByteSizeValue(0));
+        public static final AutoscalingResources ZERO = new AutoscalingResources(new ByteSizeValue(0), new ByteSizeValue(0), 0);


nit: since you're changing this line anyway you could change new ByteSizeValue(0) to ByteSizeValue.ZERO.

droberts195 · 2022-06-22T10:07:05Z

...lasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryAndProcessorInfoService.java

    private volatile TimeValue fetchTimeout;

    private final Client client;
    private final Object mutex = new Object();

    @Inject
-    public AutoscalingMemoryInfoService(ClusterService clusterService, Client client) {
+    public AutoscalingMemoryAndProcessorInfoService(ClusterService clusterService, Client client) {


Not something for this PR, but this class is doing the same thing as https://github.com/elastic/elasticsearch/pull/87366/files#diff-d71d08cc6d198f07a28ee2e1179053197941de85851c6797f30a516af523d158R725-R726 in a different way. So we have two different plugins that both want to know how much memory and how many processors other nodes have and are getting that information in two completely different ways. It might be nice to consolidate this into core server at some point in the future. If each node provided its memory and processors when joining the cluster then it could be available in cluster state - effectively like what ML is doing with node attributes but integrated as a first-class feature.

henningandersen

I would like to see a REST (yml) test added as well validating that we see processors in the current capacity output as well as that fixed decider can produce a processors output, see "get_autoscaling_capacity.yml".

Left a number of detailed other comments. The naming is tricky. But just a name ofc... Otherwise looking good.

...n/java/org/elasticsearch/xpack/autoscaling/action/TransportGetAutoscalingCapacityAction.java

...oscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingCapacity.java

...lasticsearch/xpack/autoscaling/capacity/memory/AutoscalingMemoryAndProcessorInfoService.java

...src/main/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderService.java

...g/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/memory/MemoryAndProcessors.java

...c/test/java/org/elasticsearch/xpack/autoscaling/capacity/AutoscalingDeciderResultsTests.java

...t/java/org/elasticsearch/xpack/autoscaling/capacity/FixedAutoscalingDeciderServiceTests.java

...st/java/org/elasticsearch/xpack/autoscaling/storage/ReactiveStorageDeciderDecisionTests.java

…m:benwtrent/elasticsearch into feature/add-processors-to-autoscaling-api

elasticmachine · 2022-06-22T16:01:26Z

Pinging @elastic/clients-team (Team:Clients)

henningandersen

LGTM

henningandersen · 2022-06-27T09:49:11Z

...utoscaling/src/main/java/org/elasticsearch/xpack/autoscaling/capacity/nodeinfo/NodeInfo.java

+ * @param memory node total memory
+ * @param processors allocated processors
+ */
+public record NodeInfo(long memory, float processors) {


NodeInfo is fine, but I'd prefer to add Autoscaling prefix like on the service. AutoscalingNodeInfo can become AutoscalingNodeInfos or AutoscalingNodesInfo instead.

henningandersen · 2022-06-27T12:28:02Z

...c/main/java/org/elasticsearch/xpack/autoscaling/capacity/FixedAutoscalingDeciderService.java

+                "fixed storage [%s] memory [%s] nodes [%d]%s",
+                storage,
+                memory,
+                nodes,
+                Optional.ofNullable(processors).map(i -> " processors [" + i + "]").orElse("")


Let us keep nodes at then end instead. There is no API compatibility guarantee on this message.

Suggested change

"fixed storage [%s] memory [%s] nodes [%d]%s",

storage,

memory,

nodes,

Optional.ofNullable(processors).map(i -> " processors [" + i + "]").orElse("")

"fixed storage [%s] memory [%s] processors [%s] nodes [%d]",

storage,

memory,

processors,

nodes

@henningandersen this technically breaks our REST compatibility as the reason for old API calls is now different.

It is a change to a message field, like a log message output. We make many such changes. The individual fields are available for direct consumption too, there is no need for clients to parse this field.

Another example is allocation explain, where we have refined the messaging several times without calling that a breaking change.

The rest tests probably shouldn't do a perfect string match on this field anyways...I will mute them for this PR and update.

…ssors-to-autoscaling-api

benwtrent · 2022-06-28T12:44:00Z

@elasticmachine update branch

benwtrent · 2022-06-28T13:43:49Z

@elasticmachine update branch

Add documentation for new `processors` response in autoscaling capacity API. Relates elastic#87895

Add documentation for new `processors` response in autoscaling capacity API. Relates #87895

Add processors to autoscaling capacity response

c74d372

benwtrent added >enhancement :Distributed Coordination/Autoscaling cloud-deploy Publish cloud docker image for Cloud-First-Testing v8.4.0 labels Jun 21, 2022

elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Jun 21, 2022

Update docs/changelog/87895.yaml

f60f8e6

benwtrent requested review from henningandersen and droberts195 and removed request for henningandersen June 21, 2022 18:44

droberts195 approved these changes Jun 22, 2022

View reviewed changes

henningandersen reviewed Jun 22, 2022

View reviewed changes

benwtrent added 3 commits June 22, 2022 10:37

addressing pr comments

c478fa4

renaming

ba35cdd

Merge branch 'feature/add-processors-to-autoscaling-api' of github.co…

c8e70f9

…m:benwtrent/elasticsearch into feature/add-processors-to-autoscaling-api

sethmlarson added the Team:Clients Meta label for clients team label Jun 22, 2022

benwtrent requested a review from henningandersen June 22, 2022 16:22

barkbay mentioned this pull request Jun 23, 2022

Autoscaling based on processors elastic/cloud-on-k8s#5823

Closed

benwtrent added 3 commits June 23, 2022 08:12

switch processors to a float

e3bd75d

fixing line

3bdf22f

fixing parameter name

83c9c93

henningandersen approved these changes Jun 27, 2022

View reviewed changes

benwtrent added 3 commits June 27, 2022 13:19

Merge remote-tracking branch 'upstream/master' into feature/add-proce…

31ef5d6

…ssors-to-autoscaling-api

addressing pr comments

65d65a0

Merge remote-tracking branch 'upstream/master' into feature/add-proce…

56086b4

…ssors-to-autoscaling-api

benwtrent added 3 commits June 27, 2022 14:50

fixing tests

1a9eb77

fixing format

6d6d6ed

muting test

a58d21d

Merge branch 'master' into feature/add-processors-to-autoscaling-api

b5c508a

Merge branch 'master' into feature/add-processors-to-autoscaling-api

dd6777c

benwtrent merged commit 164decb into elastic:master Jun 28, 2022

benwtrent deleted the feature/add-processors-to-autoscaling-api branch June 28, 2022 14:46

henningandersen mentioned this pull request Jul 4, 2022

[DOCS] Document autoscaling processors #88248

Merged

henningandersen added a commit to henningandersen/elasticsearch that referenced this pull request Jul 4, 2022

[DOCS] Document autoscaling processors

b9ead66

Add documentation for new `processors` response in autoscaling capacity API. Relates elastic#87895

henningandersen added a commit that referenced this pull request Jul 5, 2022

[DOCS] Document autoscaling processors (#88248)

8779330

Add documentation for new `processors` response in autoscaling capacity API. Relates #87895

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add processors to autoscaling capacity response #87895

Add processors to autoscaling capacity response #87895

benwtrent commented Jun 21, 2022

elasticmachine commented Jun 21, 2022

elasticsearchmachine commented Jun 21, 2022

benwtrent commented Jun 21, 2022

droberts195 left a comment

droberts195 Jun 22, 2022

droberts195 Jun 22, 2022

henningandersen left a comment

elasticmachine commented Jun 22, 2022

henningandersen left a comment

henningandersen Jun 27, 2022

henningandersen Jun 27, 2022

benwtrent Jun 27, 2022 •

edited

Loading

henningandersen Jun 27, 2022

benwtrent Jun 27, 2022

benwtrent commented Jun 28, 2022

benwtrent commented Jun 28, 2022


		public static final AutoscalingResources ZERO = new AutoscalingResources(new ByteSizeValue(0), new ByteSizeValue(0));
		public static final AutoscalingResources ZERO = new AutoscalingResources(new ByteSizeValue(0), new ByteSizeValue(0), 0);

Add processors to autoscaling capacity response #87895

Add processors to autoscaling capacity response #87895

Conversation

benwtrent commented Jun 21, 2022

elasticmachine commented Jun 21, 2022

elasticsearchmachine commented Jun 21, 2022

benwtrent commented Jun 21, 2022

droberts195 left a comment

Choose a reason for hiding this comment

droberts195 Jun 22, 2022

Choose a reason for hiding this comment

droberts195 Jun 22, 2022

Choose a reason for hiding this comment

henningandersen left a comment

Choose a reason for hiding this comment

elasticmachine commented Jun 22, 2022

henningandersen left a comment

Choose a reason for hiding this comment

henningandersen Jun 27, 2022

Choose a reason for hiding this comment

henningandersen Jun 27, 2022

Choose a reason for hiding this comment

benwtrent Jun 27, 2022 • edited Loading

Choose a reason for hiding this comment

henningandersen Jun 27, 2022

Choose a reason for hiding this comment

benwtrent Jun 27, 2022

Choose a reason for hiding this comment

benwtrent commented Jun 28, 2022

benwtrent commented Jun 28, 2022

benwtrent Jun 27, 2022 •

edited

Loading