0

I have this code for managing dashboard which contains approximate 100 of independent checks.

Check results are received via AJAX call.

There is one initial request for each check at start. After result is received for particular check, the code recursively waits for set timeout and repeats the request again for that check again.

One promise = one check.

I am wondering why promises starts to resolve only after each one of them is pending (none of them is in timeout period). And that is even if response from server is "instantaneous", they just wait for the last promise in cycle.

const TIMEOUT = 4000;

function checkForUpdate(environment, application, check) {
    Dashboard.setCheckPending(environment, application, check);

    return Communicator.getStatus(environment, application, check)
        .then(status => {
            Dashboard.updateCheckCell(environment, application, check, status);
            Dashboard.updateEnvironmentCell(environment, application);

            setTimeout(() => {
                    return checkForUpdate(environment, application, check)
                },
                TIMEOUT
            );
        });
}


Communicator.getEnvMatrix()
    .then(data => {
        Dashboard.create(data);

        $.each(data, (environment, applications) => {
            $.each(applications, (application, checks) => {
                $.each(checks, (key, check) => {
                    checkForUpdate(environment, application, check);
                });
            });
        });
    });

The question is also how to rewrite that so each of the checks waits just for its own result to be delivered and for set timeout.


EDIT (clarification):

Each of the 100 checks are independent, that is why I want to run AJAX for each of them as soon as I can (inside the $.each() loops).

The check is dependent only on itself. I don't want it to wait on any other check.

After the result of a check is received it has to wait for set timeout before it tries to retrieve its status again. That is why I encapsulated the recursive function within the setTimeout().

Even if I rewrite (see below) the setTimeout() as promise, the behavior stays the same unfortunately.

function delay(timeout) {
    return new Promise(resolve => {
        setTimeout(resolve, timeout);
    });
}


function checkForUpdate(environment, application, check) {
    Dashboard.setCheckPending(environment, application, check);

    let promise = Communicator.getStatus(environment, application, check).promise();

    return promise
        .then(status => {
            Dashboard.updateCheckCell(environment, application, check, status);
            Dashboard.updateEnvironmentCell(environment, application);

            return delay(TIMEOUT).then(() => {
                return checkForUpdate(environment, application, check);
            });
        });
}
meridius
  • 1,158
  • 1
  • 14
  • 26
  • 1
    Why are you not using resolve () instead of Timeout – karthik006 Jul 04 '17 at 15:05
  • 1
    Can you describe better what this code is supposed to accomplish? There are several things wrong here, but I don't quite follow what the objective of this code is to know how to suggest a proper alternative. For example, you are calling `checkForUpdate()` after a timeout, but that's completely independent of all previous promises so it's unrelated to any of that and you appear to be doing that forever. Is that really what you want? – jfriend00 Jul 04 '17 at 15:08
  • What version of jQuery are you running? jQuery promises behave differently in different versions. Are these promises jQuery promises or a more standards-comforming promise? – jfriend00 Jul 04 '17 at 15:11
  • @karthik006 I need to wait for specific time after check result is received. This way I won't repeat the request right after the response was received. That is why I used `setTimeout`. – meridius Jul 04 '17 at 15:11
  • @jfriend00 `checkForUpdate()` is intentionally called recursively and after a timeout. Please see my response above. – meridius Jul 04 '17 at 15:14
  • 1
    Is the recursive `checkForUpdate()` supposed to be chained onto the original promise or just starting a new independent promise chain? – jfriend00 Jul 04 '17 at 15:17
  • Your code is designed to have all the `checkForUpdate()` in flight at the same time. Is that what you want? Or, do you want to run one to completion, then run the next one and so on? Can't help you better without understanding what this code is supposed to do. Please go back to first principles and describe in words what you are trying to accomplish with your code, edit that into your question and drop a comment that you've edited the question. – jfriend00 Jul 04 '17 at 15:24
  • Could you be more precise about what is unexpected, please? How do you know that "a promise starts to resolve" or "is pending", do you use logging? What exact statements do you expect to happen in what order? – Bergi Jul 04 '17 at 15:50
  • @Bergi The unexpected thing is that even if all 100 requests are sent immediately after page loads, they are being resolved just few at a time and (roughly) in the order they've been sent. Roughly means `3, 2, 5, 1, 8, ...`. But I'd expect something like `3, 89, 12, 76, 21, 94, ...`. There seems to be some limit on how many promises can be run concurrently and in what order. – meridius Jul 04 '17 at 16:36
  • @Bergi And as I wrote in the question, the code handles dashboard. So I can see when the request was made and when response came. Also I can see that in browser's console. – meridius Jul 04 '17 at 16:38
  • 1
    There is a limit on how many ajax calls the browser will run concurrently to the same host so subsequent calls will be queued until earlier ones finish. Browsers vary on exactly how many ajax calls they will run concurrently to the same host, but it's a relatively small number (like under 10). So, if your `.each()` loop is starting a hundred ajax calls to the same host, you will hit the limit. – jfriend00 Jul 04 '17 at 16:38
  • 1
    And, if that's your real question, why don't you put that into the question so we can directly address that? We can help you better if you are much clearer on exactly what you observe and exactly what you expect. Here we are an hour later and lots of back and forth and only now are you describing what you actually observed and what you expected. Really hard to help when you hold that info back. – jfriend00 Jul 04 '17 at 16:42
  • @meridius That's nothing to do with promises (which already resolve out of order as you see), but a concurrency limit on open network connections that your browser imposes on ajax requests. Oh, jfriend already commented on that. – Bergi Jul 04 '17 at 16:45
  • @jfriend00 Limit on number of AJAX calls seems to be the real culprit here. I'm sorry I didn't write that in the question right away but I didn't know that when I wrote it. I thought my problem was more promises-related and that's why I wrote the question that way. – meridius Jul 04 '17 at 16:46
  • 1
    @meridius - The key to good question writing is to not make too many assumptions about what is causing the problem. If you are wrong in those assumptions, then you don't get a good answer for a long time because your question is incomplete. Instead, describe what you observed in detail and describe what you expected and we can directly address that without your assumptions clouding what you decided to describe. – jfriend00 Jul 04 '17 at 16:48

1 Answers1

2

Your code runs $.each() synchronously. That means it is going to call every checkForUpdate() before anything else can run. Since standards-conforming promises are always resolved asynchronously (on some future tick), that means that every single request here will get started before ANY promise can run its .then() handler. That's how promises work. Only once the $.each() loop is done can the Javascript interpreter start to process the .then() handlers of resolved promises.

Also, it is unclear why you are trying to do a return checkForUpdate(environment, application, check) inside the setTimeout(). The return there does nothing. It's just returning to the setTimeout() callback which does nothing. The parent function has long since already returned so this is not chaining the next checkForUpdate() to the previous promise chain. If you wanted to chain them together, then you need to make a delay with a promise and return that promise like is shown in these references:

using setTimeout on promise chain

Delays between promises in promise chain

Delay chained promise


The unexpected thing is that even if all 100 requests are sent immediately after page loads, they are being resolved just few at a time and (roughly) in the order they've been sent. Roughly means 3, 2, 5, 1, 8, .... But I'd expect something like 3, 89, 12, 76, 21, 94, .... There seems to be some limit on how many promises can be run concurrently and in what order.

Another thing that will influence your ajax calls is that each browser has a limit on how many concurrent ajax calls it will allow to the same host. If you exceed that limit, it will queue them and not run subsequent ones until some earlier ones finish. Each browser sets its own limit and they've changed over time so I don't know exactly what the current limits are, but they are lowish. I know Chrome used to be something like 6 at at time to the same host. So, that will also affect the exact order that things complete.

When you hit this limit, Ajax calls will be sent in the order they were called by your code. So, if the limit was 6 per host, then your first 6 would be sent and the 7th request would only be sent when one of the first 6 finished and so on. That still doesn't guarantee a finish order, but it does affect the ability for a later request to finish before an earlier request.

jfriend00
  • 580,699
  • 78
  • 809
  • 825
  • Added comments about the `setTimeout()` delay. – jfriend00 Jul 04 '17 at 15:24
  • Please see clarification in edit of the question and let me know if there is anything I can clarify more. – meridius Jul 04 '17 at 15:44
  • @meridius - Does my answer not explain what you are seeing? What else are you looking for an explanation of that my answer does not describe? It sounds like you want to run all these checks at once from the `$.each()` loop so that's what your code is doing. The `.then()` handlers cannot run until the `$.each()` loop has finished starting all the operations. That's how Javascript and promises work. If you want it to behave differently, please describe what you want that it is not doing. – jfriend00 Jul 04 '17 at 15:48
  • @meridius - Added info about browser ajax concurrent connection limits to the same host. – jfriend00 Jul 04 '17 at 16:45