Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase default pagination size #10

Closed
toolmantim opened this issue Nov 15, 2016 · 16 comments
Closed

Increase default pagination size #10

toolmantim opened this issue Nov 15, 2016 · 16 comments

Comments

@toolmantim
Copy link
Contributor

With lots of agents and a few stacks, it's easy to go over 100RPM on agents/list pagination.

We should up the page size to be 100. The default is 30.

@lox
Copy link
Contributor

lox commented Nov 15, 2016

Sounds very sensible.

@keithpitt
Copy link
Member

Without digging too much into the code, what data do you need from the Agents List? Is it looking for stuff on queue=x things?

@toolmantim
Copy link
Contributor Author

@keithpitt it just gets the agent meta_data and checks if job != nil to see if it's "busy"

@toolmantim
Copy link
Contributor Author

It's all the agent.* calls in this code

@keithpitt
Copy link
Member

Ah right, is there anything I could do to convince you to use the GraphQL API? :) Or would that be too hard basket? So we have queue search in the API:

query {
  organization() {
    agents(metaData: "queue=lol") {
      job { ... }
    }
  }
}

@toolmantim
Copy link
Contributor Author

@keithpitt yeah, I don't see why not! People's API tokens would need changing tho?

We don't need to do queue search tho, we just want to grab all the agents… and then we loop through the list and collate them by queue and whether or not they have a job assigned.

@keithpitt
Copy link
Member

I was just worried that since the Agents API endpoint has a really slow pagination mechanism, and includes all the things.

Ah yeah...the API tokens would need to change.

Maybe too hard basket for now...

@toolmantim
Copy link
Contributor Author

Say you had 45 agents with 3 queues. What if we add an endpoint to the Agents REST API

curl "https://api.buildkite.com/v2/organizations/{org.slug}/agents/queue-metrics"

which would return…

{
  "count": 45,
  "busy": 20,
  "idle": 25,
  "queues": {
    "default": {
      "count": 20,
      "busy": 15,
      "idle": 5
    },
    "osx": {
      "count": 10,
      "busy": 2,
      "idle": 8
    },
    "elastic": {
      "count": 15,
      "busy": 12,
      "idle": 3
    }
  }
}

@keithpitt
Copy link
Member

That could be a pretty straight forward SQL call as well - no pagination required! I'm in favour!

@lox
Copy link
Contributor

lox commented Nov 16, 2016

I'd love this repo to not be a thing.

@lox
Copy link
Contributor

lox commented Nov 16, 2016

I hate things.

@keithpitt
Copy link
Member

What would it be if it wasn't a thing?!

@lox
Copy link
Contributor

lox commented Nov 16, 2016

Yeah, fair point, unless you guys ended up pushing metrics directly into Cloudwatch it's going to stick around. I'd be perfectly happy to talk to a different less brute force endpoint or GraphQL. I tried that first, but it was very early days @keithpitt.

@keithpitt
Copy link
Member

ended up pushing metrics directly into Cloudwatch

That's something I've always thought would be a good future change, but I figured it'd be too hard. In saying that though, I only think it's hard mostly because I've never actually asked what it would entail!

Is it a matter of just sending queue metrics default: 1 busy, 3 idle type stuff to Cloudwatch?

@lox
Copy link
Contributor

lox commented Nov 17, 2016

Yeah, probably too hard.

Providing a better API for these metrics sounds like the bigger win, or at
least exposing them via GraphQL.

On Thu, Nov 17, 2016 at 8:17 AM Keith Pitt <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

ended up pushing metrics directly into Cloudwatch

That's something I've always thought would be a good future change, but I
figured it'd be too hard. In saying that though, I only think it's hard
mostly because I've never actually asked what it would entail!

Is it a matter of just sending queue metrics default: 1 busy, 3 idle type
stuff to Cloudwatch?


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#10 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAA9jm4koOnmkcmWf-REXUhaIsuoQzG2ks5q-3LhgaJpZM4KzHn6
.

@lox
Copy link
Contributor

lox commented Nov 26, 2016

Fixed in master.

@lox lox closed this as completed Nov 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants