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

[proxy] Fix proxy to be able to re-send request body #5361

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

addisonj
Copy link
Contributor

Fixes #5360

This adds a small cache of the request body to ensure that it can be
re-sent.

@addisonj addisonj force-pushed the fix_proxy branch 2 times, most recently from 3ff54c8 to 3abfe71 Compare October 11, 2019 07:00
@addisonj
Copy link
Contributor Author

Okay, confirmed this works... not sure of an easy way to unit test it. I also want to add more one more change here to fix the strack trace on redirects that is confusing

@addisonj
Copy link
Contributor Author

rerun integration tests
rerun cpp tests
rerun java8 tests

@merlimat
Copy link
Contributor

not sure of an easy way to unit test it

I think it should be happening each time the proxy is handling a 307 and the request has a body.
In

// Test multiple stats request to make sure the proxy will try against all brokers and receive 307
// responses that it will handle internally.
for (int i = 0; i < 10; i++) {
admin.topics().getStats(topic);
}

I was forcing multiple GET request to the proxy. The idea is that the proxy will round-robin the requests on the available brokers.

We could do the same for a PUT/POST request.

@merlimat merlimat added this to the 2.4.2 milestone Oct 11, 2019
@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Oct 11, 2019
@merlimat
Copy link
Contributor

run java8 tests

@addisonj
Copy link
Contributor Author

run java8 tests

Fixes apache#5360

This adds a small cache of the request body to ensure that it can be
re-sent.

TODO: still needs tested
@addisonj
Copy link
Contributor Author

rerun integration tests
rerun cpp tests

@sijie
Copy link
Member

sijie commented Oct 24, 2019

run java8 tests
run integration tests

@merlimat merlimat merged commit 978efaf into apache:master Oct 24, 2019
wolfstudy pushed a commit that referenced this pull request Nov 20, 2019
Fixes #5360

This adds a small cache of the request body to ensure that it can be
re-sent.

TODO: still needs tested
(cherry picked from commit 978efaf)
@rivernate rivernate deleted the fix_proxy branch October 30, 2020 20:31
sijie pushed a commit that referenced this pull request Jun 22, 2021
Fixes #10908

### Motivation

Pulsar Proxy uses a lot of heap memory when uploading large function jar files. This also leads to high GC activity since a continuous block of memory (byte array for the size of the upload) is allocated. GC will have to do compaction for the heap (which gets fragmented) to find a continuous block of memory. This is the reason why allocating large arrays are costly from GC perspective. 

The buffering solution added as part of #5361. The solution buffers also very large uploads to memory.

### Modifications

* Limit the replay buffer size to a configurable limit which defaults to 5MB. This is configured with the `httpInputMaxReplayBufferSize` proxy configuration parameter.
* Add unit test to see that buffer size gets limited
* Add unit test for #5361
eolivelli pushed a commit to datastax/pulsar that referenced this pull request Jun 23, 2021
Fixes apache#10908

### Motivation

Pulsar Proxy uses a lot of heap memory when uploading large function jar files. This also leads to high GC activity since a continuous block of memory (byte array for the size of the upload) is allocated. GC will have to do compaction for the heap (which gets fragmented) to find a continuous block of memory. This is the reason why allocating large arrays are costly from GC perspective.

The buffering solution added as part of apache#5361. The solution buffers also very large uploads to memory.

### Modifications

* Limit the replay buffer size to a configurable limit which defaults to 5MB. This is configured with the `httpInputMaxReplayBufferSize` proxy configuration parameter.
* Add unit test to see that buffer size gets limited
* Add unit test for apache#5361

(cherry picked from commit 2324618)
yangl pushed a commit to yangl/pulsar that referenced this pull request Jun 23, 2021
Fixes apache#10908

### Motivation

Pulsar Proxy uses a lot of heap memory when uploading large function jar files. This also leads to high GC activity since a continuous block of memory (byte array for the size of the upload) is allocated. GC will have to do compaction for the heap (which gets fragmented) to find a continuous block of memory. This is the reason why allocating large arrays are costly from GC perspective. 

The buffering solution added as part of apache#5361. The solution buffers also very large uploads to memory.

### Modifications

* Limit the replay buffer size to a configurable limit which defaults to 5MB. This is configured with the `httpInputMaxReplayBufferSize` proxy configuration parameter.
* Add unit test to see that buffer size gets limited
* Add unit test for apache#5361
kaushik-develop pushed a commit to kaushik-develop/pulsar that referenced this pull request Jun 24, 2021
Fixes apache#10908

### Motivation

Pulsar Proxy uses a lot of heap memory when uploading large function jar files. This also leads to high GC activity since a continuous block of memory (byte array for the size of the upload) is allocated. GC will have to do compaction for the heap (which gets fragmented) to find a continuous block of memory. This is the reason why allocating large arrays are costly from GC perspective. 

The buffering solution added as part of apache#5361. The solution buffers also very large uploads to memory.

### Modifications

* Limit the replay buffer size to a configurable limit which defaults to 5MB. This is configured with the `httpInputMaxReplayBufferSize` proxy configuration parameter.
* Add unit test to see that buffer size gets limited
* Add unit test for apache#5361
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Feb 28, 2022
Fixes apache#10908

Pulsar Proxy uses a lot of heap memory when uploading large function jar files. This also leads to high GC activity since a continuous block of memory (byte array for the size of the upload) is allocated. GC will have to do compaction for the heap (which gets fragmented) to find a continuous block of memory. This is the reason why allocating large arrays are costly from GC perspective.

The buffering solution added as part of apache#5361. The solution buffers also very large uploads to memory.

* Limit the replay buffer size to a configurable limit which defaults to 5MB. This is configured with the `httpInputMaxReplayBufferSize` proxy configuration parameter.
* Add unit test to see that buffer size gets limited
* Add unit test for apache#5361

(cherry picked from commit 2324618)
(cherry picked from commit 7fa88cc)
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
Fixes apache#10908

### Motivation

Pulsar Proxy uses a lot of heap memory when uploading large function jar files. This also leads to high GC activity since a continuous block of memory (byte array for the size of the upload) is allocated. GC will have to do compaction for the heap (which gets fragmented) to find a continuous block of memory. This is the reason why allocating large arrays are costly from GC perspective. 

The buffering solution added as part of apache#5361. The solution buffers also very large uploads to memory.

### Modifications

* Limit the replay buffer size to a configurable limit which defaults to 5MB. This is configured with the `httpInputMaxReplayBufferSize` proxy configuration parameter.
* Add unit test to see that buffer size gets limited
* Add unit test for apache#5361
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proxy doesn't send request body on redirects
3 participants