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

Blocking HTTP Request #64

Closed
vanvanni opened this issue Nov 11, 2019 · 6 comments
Closed

Blocking HTTP Request #64

vanvanni opened this issue Nov 11, 2019 · 6 comments

Comments

@vanvanni
Copy link

The api receives 3mb of logs pushed to the push uri. This data will be added to the queue but does push wait till it is finished? Cause my api reponse will not accur?


    const APP_LOG = req.params.app.toLowerCase();
    let BODY: String = req.body;
    if (BODY.endsWith(";;")) {
        BODY = BODY.trim();
        BODY = BODY.substring(0, BODY.length - 2);
    }
    let lines = BODY.split(";;");

    que.push({
        app: APP_LOG,
        lines
    });

And for my queue:

let que = new BetterQueue(async function (input, cb) {
    console.time(`Queue`);
// Here I put the logs in the DB
    console.timeEnd(`Queue`);
    cb();
});
@nmargaritis
Copy link

This probably does not have to do with better-queue. Try to call this cb(); in either the http request callback or promise.then depending on what you are using.

request.post({url:'http://service.com/upload', formData: formData}, function optionalCallback(err, httpResponse, body) {
  if (err) {
    return console.error('upload failed:', err);
  }
  console.log('Upload successful!  Server responded with:', body);
  cb(); // <-----------------------------
});

@vanvanni
Copy link
Author

vanvanni commented Nov 12, 2019

I mean that the post request waits till the queue is done. Since when I push 250kb of data is is done reallly fast but when I do 10mb it takes ages. And I get the reponse back from the Express server when better queue has done all the data queued

$request = curl_init();

        curl_setopt_array($request, [
            CURLOPT_PORT => 3030 ,
            CURLOPT_URL => $this->destination,
            CURLOPT_RETURNTRANSFER => true,
            CURLOPT_TIMEOUT => 30,
            CURLOPT_HTTP_VERSION => CURL_HTTP_VERSION_1_1,
            CURLOPT_CUSTOMREQUEST => "POST",
            CURLOPT_POSTFIELDS => $textBody,
            CURLOPT_HTTPHEADER => [
                "content-type: text/plain"
            ]
        ]);

        $response = curl_exec($request);
        $err = curl_error($request);
        curl_close($request);

        if ($err != "") {
            throw new \ErrorException("CURL Error: " . $err);
            return;
        }

        $statusJson = json_decode($response);
        if ($statusJson->status != "PUSHED") {
            throw new \ErrorException("Field pushing logs to server: " . $statusJson->data);
            return;
        }

@nmargaritis
Copy link

nmargaritis commented Nov 12, 2019

I mean that the post request waits till the queue is done

This is again not related with better-queue.
It is an implementation detail.

Your endpoints should not wait for the queue to process the task, otherwise you are not really using the queue.

A practice that is followed is that; for long tasks, you return 202 Accepted as a status code with a ticketId (the ticketid is returned from better queue upon task creation). Then you can store this ticket id somewhere and know when tasks are done or not, at a later point, but your request only waits for the ticketId and not the actual processing.

It really depends on your implementation and usage, but consider that the way you have implemented this, if N users are using your express server to upload stuff, the one will wait for the other cause you are queuing and then you are waiting from a single queue to finalise a task. Some users will probably timeout or wait for minutes.

The queue concept is meant to help you cope with the load and heavy processing. But user responses should not wait until you do that, you can simply say that I have accepted a task, with this ticket id and provide another endpoint that simply checks this ticket id for completion. There are other approaches as well, but nonetheless this question is not related with this library.

@vanvanni
Copy link
Author

@nmargaritis How should I do that? This is the function behind the API Route:

if (req.params.app == undefined) {
        return res.json({
            status: "MISSING_DATA",
            message: "Missing app name for logs"
        });
    }

    const APP_LOG = req.params.app.toLowerCase();
    let BODY: String = req.body;
    if (BODY.endsWith(";;")) {
        BODY = BODY.trim();
        BODY = BODY.substring(0, BODY.length - 2);
    }
    let lines = BODY.split(";;");

    que.push({
        app: APP_LOG,
        lines
    });

    return res.json({
        status: "PUSHED",
        data: {
            lines: lines.length
        }
    });

@nmargaritis
Copy link

nmargaritis commented Nov 15, 2019

@xJoeyv In the given example, you are not waiting for the queue to finish processing as you were previously suggesting.

You are just pushing a task to it. Your logic of parsing body outside of the queue, is what causes your API response to be slow., the bigger the body, the slower. It would make sense to move all the heavy logic to the queue handler and reconsider the way you evaluate body. You are splitting MBs just to return the lines length.

Try it.

    console.time('parsingBody');
    if (req.params.app == undefined) {
        return res.json({
            status: 'MISSING_DATA',
            message: 'Missing app name for logs'
        });
    }

    const APP_LOG = req.params.app.toLowerCase();
    let BODY: String = req.body;
    if (BODY.endsWith(';;')) {
        BODY = BODY.trim();
        BODY = BODY.substring(0, BODY.length - 2);
    }
    let lines = BODY.split(';;');
    console.timeEnd('parsingBody');
    console.time('pushingToQueue');
    que.push({
        app: APP_LOG,
        lines
    });
    console.timeEnd('pushingToQueue');
    return res.json({
        status: 'PUSHED',
        data: {
            lines: lines.length
        }
    });

Note that these issues on GitHub are related with this node module and its problems Since your problems are not related with better-queue, please use the appropriate means if you have such questions, such as stackoverflow. I wont be assisting on any other question, unrelated to better-queue.

@vanvanni
Copy link
Author

Alright thanks! Issue can be closed!

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

2 participants