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

Error: http: request body too large (recovered by wazero) #673

Open
mtb0x1 opened this issue Feb 6, 2024 · 5 comments
Open

Error: http: request body too large (recovered by wazero) #673

mtb0x1 opened this issue Feb 6, 2024 · 5 comments

Comments

@mtb0x1
Copy link

mtb0x1 commented Feb 6, 2024

Command to reproduce :

cli --allow-host "cdimage.debian.org" call wasm/http.wasm http_request --input '{"url": "https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-12.4.0-amd64-netinst.iso"}' > debian.iso

Result :

Error: http: request body too large (recovered by wazero)
wasm stack trace:
        extism:host/env.http_request(i64,i64) i64
        ._ZN10extism_pdk4http7request17h4b0709172c155b02E(i32,i32,i32,i32)
        .http_request() i32

Expected Result :

iso file downloaded entirely, if not a warning/error message explaining the reason.

in runtime/src/pdk.rs, there is a limit set to 50MB (?).

extism/runtime/src/pdk.rs

Lines 240 to 242 in 1a083f6

reader
.take(1024 * 1024 * 50) // TODO: make this limit configurable
.read_to_end(&mut buf)?;

My question I guess is, beyond the TODO to configure the limit, is there a hard-limit that we should worry about either due to Extism or WASI design.

@bhelx bhelx assigned mhmd-azeez and unassigned mhmd-azeez Feb 6, 2024
@zshipko
Copy link
Contributor

zshipko commented Feb 7, 2024

I just opened a PR to update the CLI to add the --http-response-max: extism/cli#64 after adding the new manifest field: #674

Other than that the only other limit I can think of is the 32-bit address space. Also, this setting only applies to calls to extism_http_request, it doesn't have any affect on any WASI configuration.

@mtb0x1
Copy link
Author

mtb0x1 commented Feb 8, 2024

Now I get a random signal : killed, monitoring the execution using 'top' the memory goes up to ~5gb. (my vm has 6gb max)
image

Steps :

  1. Clone extism/extism,extism/cli,extism/rust-pdk,extism/plugins into /tmp
  2. Change /tmp/plugins/Cargo.toml to use /tmp/rust-pdk instead of git version.
  3. Change /tmp/rust-pdk/Cargo.toml to use /tmp/extism/manifest and /tmp/extism/convert instead of public version.
    (the /tmp/extism is the last version from git)
  4. Update /tmp/cli with PR 'http-response-max' gh pr checkout 64
  5. Build plugins cd /tmp/plugins && cargo build
  6. Run go run /tmp/cli/extism/main.go --allow-host "cdimage.debian.org" --http-response-max 1048576000 call /tmp/plugins/target/wasm32-unknown-unknown/debug/http.wasm http_get --input '{"url": "https://cdimage.debian.org/debian-cd/current/amd64/iso-cd/debian-12.4.0-amd64-netinst.iso"}' > debian.iso
    (1048576000 ~= 1gb)

from what I gathered --http-response-max xyz works fine, when I specify 524288000 (~=500Mb) as a value I get the usual request body too large. (the iso size is ~700Mb)

I ran a quick check and I might be missing things, my guess the excessive memory usage happens in

pub fn memory_new<'a, T: ToBytes<'a>>(&mut self, t: T) -> Result<MemoryHandle, Error> {

// +1gb allocated in `http_request`, then in 
    pub fn memory_new<'a, T: ToBytes<'a>>(&mut self, t: T) -> Result<MemoryHandle, Error> {
        let data = t.to_bytes()?; //<= +700mb ?
        let data = data.as_ref();
        if data.is_empty() {
            return Ok(MemoryHandle::null());
        }
        let handle = self.memory_alloc(data.len() as u64)?; //<= +700mb ?
        let bytes = self.memory_bytes_mut(handle)?;
        bytes.copy_from_slice(data.as_ref()); //<= +700mb ?
        Ok(handle)
    }
   //=> ~4gb and I am probably missing out some other allocations.

I didn't have time to run Valgrind to confirm, in any case, this is not urgent but I think it's worth following up as there is room for improvement that would benefit the memory management and speed of extism runtime.

@zshipko
Copy link
Contributor

zshipko commented Feb 8, 2024

Thanks for testing it out! Your example does download the full ISO on my machine, but memory use seemed to peak at ~4GB, so there is definitely some room to improve memory use around large HTTP responses. I will investigate a little further today.

@nilslice
Copy link
Member

@mtb0x1 we merged a solution for this last week, just want to follow up to make sure you saw & see if it works for you!

@mtb0x1
Copy link
Author

mtb0x1 commented Feb 18, 2024

I tested with the change in http plugin, memory usage did improve and peak usage did go down from +4Gb to 2.7Gb.
(This is not a real use-case on my side and I am not sure if you need to spend more time on it)
You can close it :)

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

4 participants