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

Concurrency count is not updated immediately after closing a sandbox. #283

Closed
jamesmurdza opened this issue Jan 6, 2024 · 7 comments
Closed
Labels
bug Something isn't working

Comments

@jamesmurdza
Copy link
Contributor

Describe the bug
When closing and creating sandboxes quickly, the concurrency limit will be reached, the count is not updated right away.

To Reproduce

import { Sandbox } from "@e2b/sdk";
import { config } from "dotenv";
config();

// Map over an array with a fixed number of threads
// https://stackoverflow.com/a/71239408/8784402
const asyncMap = async <T, Q>(
  x: T[],
  threads: number,
  fn: (v: T, i: number, a: T[]) => Promise<Q>
) => {
  let k = 0;
  const result = Array(x.length) as Q[];
  await Promise.all(
    [...Array(threads)].map(async () => {
      while (k < x.length) result[k] = await fn(x[k], k++, x);
    })
  );
  return result;
};

// Run a given number of sandboxes concurrently
async function runTasks(count: number, concurrency: number) {
  let concurrentTasks = 0;
  await asyncMap(Array(count), concurrency, async () => {
    concurrentTasks++;
    console.log(`Number of running tasks: ${concurrentTasks}`);
    let sandbox = await Sandbox.create();
    await sandbox.close();
    concurrentTasks--;
  });
}

runTasks(40, 15);

Expected behavior
This should not give an error, because only 15 sandboxes are run concurrently.

Terminal commands & output

Number of running tasks: 1
Number of running tasks: 2
Number of running tasks: 3
Number of running tasks: 4
Number of running tasks: 5
Number of running tasks: 6
Number of running tasks: 7
Number of running tasks: 8
Number of running tasks: 9
Number of running tasks: 10
Number of running tasks: 11
Number of running tasks: 12
Number of running tasks: 13
Number of running tasks: 14
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
Number of running tasks: 15
/Users/james/code/project/node_modules/openapi-typescript-fetch/dist/cjs/fetcher.js:6
        function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
                                                                ^
ApiError: Forbidden
    at /Users/james/code/project/node_modules/openapi-typescript-fetch/dist/cjs/fetcher.js:154:23
    at Generator.throw (<anonymous>)
    at rejected (/Users/james/code/project/node_modules/openapi-typescript-fetch/dist/cjs/fetcher.js:6:65)
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  headers: Headers {
    [Symbol(headers list)]: HeadersList {
      cookies: null,
      [Symbol(headers map)]: [Map],
      [Symbol(headers map sorted)]: null
    },
    [Symbol(guard)]: 'immutable',
    [Symbol(realm)]: null
  },
  url: 'https://api.e2b.dev/instances',
  status: 403,
  statusText: 'Forbidden',
  data: {
    code: 403,
    message: "You have reached the maximum number of concurrent sandboxes (20). If you need more, please contact us at 'https://e2b.dev/docs/getting-help'"
  }
}
Copy link

linear bot commented Jan 6, 2024

@jamesmurdza
Copy link
Contributor Author

Also, adding a delay after sandbox.close helps, but only a long one. For this example, ten seconds seems to work.

@mlejva mlejva added the bug Something isn't working label Jan 7, 2024
@mlejva
Copy link
Member

mlejva commented Jan 7, 2024

Thank you for submitting the issue @jamesmurdza, this looks like a bug related to the fact that when you call sandbox.close() we internally keep the sandbox running for 15 seconds to finish any I/O (especially if any logs need to be send to the client).

I think we'll need to rethink how we're handling the close() process.

@jamesmurdza
Copy link
Contributor Author

Agreed. For now, I’ll have to add a delay of 15 seconds before starting the next task.

jamesmurdza added a commit to gitwitorg/react-eval that referenced this issue Mar 1, 2024
@jamesmurdza
Copy link
Contributor Author

How we're handling this currently:

Screenshot 2024-02-29 at 7 05 06 PM

Code: gitwitorg/react-eval@f2fb246

@mlejva
Copy link
Member

mlejva commented Mar 1, 2024

Hey @jamesmurdza, we have an experimental stateless SDK in the works that has a kill method
#313

It’s stateless in a sense that you don’t need to manage Sandbox’s lifecycle in your code. Would love to learn if it would be helpful to you and any other feedback.

@ValentaTomas
Copy link
Member

The .close() method is removed in the new beta SDK (https://e2b.dev/docs/guide/beta-migration). After the sandbox expires or when you call .kill() the concurrency count should be updated almost immediately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants