| Date | Commit message (Collapse) |
|---|
|
We might as well do it since puma and thin both do(*),
and we can still do writev for now to get some speedups
by avoiding Rack::Chunked overhead.
timing runs of "curl --no-buffer http://127.0.0.1:9292/ >/dev/null"
results in a best case drop from ~260ms to ~205ms on one VM
by disabling Rack::Chunked in the below config.ru
$ ruby -I lib bin/yahns-rackup -E none config.ru
==> config.ru <==
class Body
STR = ' ' * 1024 * 16
def each
10000.times { yield STR }
end
end
use Rack::Chunked if ENV['RACK_CHUNKED']
run(lambda do |env|
[ 200, [ %w(Content-Type text/plain) ], Body.new ]
end)
(*) they can do Content-Length, but I don't think it's
worth the effort at the server level.
|
|
Clients are not able to handle persistent connections unless
the client knows the length of the response.
|
|
It's too hard to reliably test output buffering behavior
with non-default values users sometimes set; so just skip
and warn about it for now.
ref: commit dad99b5ecd93cdf0a514ff9fb51d198f8aebb188
("test/test_proxy_pass: remove buffer size tuning")
|
|
rack 2.x has some incompatible changes an deprecations; support
it but remain compatible with rack 1.x for the next few years.
|
|
Rack::Lint-compliant applications wouldn't have this problem;
but apparently public-facing Rack servers (webrick/puma/thin)
all implement this; so there is precedence for implementing
this in yahns itself.
|
|
This allows us to work transparently with our OpenSSL
workaround[*] while allowing us to reuse our non-sendfile
compatibility code. Unfortunately, this means we duplicate a
lot of code from the normal wbuf code for now; but that should
be fairly stable at this point.
[*] https://bugs.ruby-lang.org/issues/12085
|
|
We may have temporary files lingering from concurrent
multi-threaded tests in our forked child since FD_CLOFORK
does not exist :P
|
|
This is mainly to benefit curl(1) users who forget to use '-f'
to show failures. Not sure if I want to keep this change, it
seems like bloat; but Rack::ShowStatus pages are totally
overkill...
|
|
It seems unnecessary to set it at all and it's deprecated
in current Ruby trunk.
|
|
Static gzip files may not exist for symlinks, but they
could resolve to a file for which a pre-gzipped file
exists.
|
|
We must ensure we properly close connections to HTTP/1.0
backends even if we blocked writing on outgoing data.
|
|
This should make it easier to figure out where certain
errors are coming from and perhaps fix problems with
upstreams, too.
This helped me track down the problem causing public-inbox WWW
component running under Perl v5.20.2 on my Debian jessie system
to break and drop connections going through
Plack::Middleware::Deflater with gzip:
https://public-inbox.org/meta/20160607071401.29325-1-e@80x24.org/
Perl 5.14.2 on Debian wheezy did not detect this problem :x
|
|
Using a 10ms tick was too little, use 100ms instead to avoid
burning CPU. Ideally, we would not tick at all during shutdown
(we're normally tickless); but the common case could be
slightly more expensive; and shutdowns are rare (I hope).
Then, change our process title to indicate we're shutting down,
and finally, cut down on repeated log spew during shutdown and
only log dropping changes.
This mean we could potentially take 90ms longer to notice when
we can do a graceful shutdown, but oh well...
While we're at it, add a test to ensure graceful shutdowns
work as intended with multiple processes.
|
|
We can force output buffer files to a directory of our
choosing to avoid being confused by temporary files
from other tests polluting the process we care about.
|
|
OpenSSL can't handle write retries if we append to an existing
string. Thus we must preserve the same string object upon
retrying.
Do that by utilizing the underlying Wbuf class which could
already handles it transparently using trysendfile.
However, we still avoiding the subtlety of wbuf_close_common
reliance we previously used.
ref: commit 551e670281bea77e727a732ba94275265ccae5f6
("fix output buffering with SSL_write")
|
|
We can retrieve it when we actually need to create the
temporary file. This saves an ivar slot and method dispatch
parameters.
This patch is nice, unfortunately the patch which follows is
not :P
|
|
On ENAMETOOLONG and perhaps other system errors which we can do
nothing about, we should not spew a giant backtrace which could
be used as an easy DoS vector.
|
|
@busy will be reset on wbuf_write anyways, since there is no
initial data and we will always attempt to write to the socket
aggressively.
|
|
Relying on @body.close in Yahns::WbufCommon#wbuf_close_common to
resume reading the upstream response was too subtle and
potentially racy.
Instead use a new Yahns::WbufLite class which does exactly what
we want for implementing this feature, and nothing more.
|
|
This may be useful to avoid wasting resources when proxying for
an upstream which can already handle slow clients itself.
It is impossible to completely disable buffering, this merely
prevents gigantic amounts of buffering.
This may be useful when an upstream can generate a gigantic
response which would cause excessive disk I/O traffic if
buffered by yahns. An example of this would be an upstream
dynamically-generating a pack for a giant git (clone|fetch)
operation.
In other words, this option allows the upstream to react to
backpressure from slow clients. It is not recommended to
enable this unless your upstream server is capable of
supporting slow clients.
|
|
Instead, we must drop non-terminated responses since
HTTP/1.0 clients do not understand chunked encoding.
This is necessary for "ab -k" which still uses HTTP/1.0.
|
|
The ab(1) command we use for testing is limited to 20000 open
connections under Debian jessie; a perfectly reasonable limit
to avoid port exhaustion. I never noticed this limit before,
but systemd under Jessie seems to have upped the default
RLIMIT_NOFILE to 65536(!), causing ab to error out.
We don't even need 10K connections for testing,
we just need to hit *some* limit before we start expiring.
So lower the RLIMIT_NOFILE back to 1024 in the forked server
process so we can test more quickly without running out of
ports or memory, since exhausting the 65536 RLIMIT_NOFILE
limit is not going to happen with a single TCP address.
|
|
These are followups to the following two commits:
* commit d16326723d
("proxy_http_response: fix non-terminated fast responses, too")
* commit 8c9f33a539
("proxy_http_response: workaround non-terminated backends")
|
|
We should not infinite loop, oops :x
Also, ensure 'yahns' is in the directory in case tests are
SIGKILL-ed and directories are left over.
|
|
We need to ensure SERVER_PORT is still parsed from the Host:
header when it is given, there.
|
|
This helps Rack::Request#url and similar methods generate proper
URLs instead of the obviously wrong: "https://example.com:80/"
Note: we don't track the actual port the listener is bound to,
and it may not be worth it since the use of the Host: header
is long-established and Host: headers include the port number
if non-standard.
|
|
The "threads:" option for the "listen" directive is worthless.
Having a dedicated thread per-process is already more than enough
(and ideal) for a multi-process setup. Multiple acceptor threads
is still wrong for a single-process setup (even if we did not
have a GVL) as it still incurs contention with the worker
pool within the kernel.
So remove the documentation regarding "listen ... threads: ",
for now; at least until somebody can prove it's useful and not
taking up space.
Additionally, "atfork_parent" may be useful for restarting
background threads/connections if somebody wants to run
background jobs in the master process, so stop saying
it's completely useless.
|
|
env['HTTPS'] is not documented in rack SPEC, but appears to be
used by Rack::Request since 2010[*]. Also, set rack.url_scheme
as documented by rack SPEC.
[*] - commit 4defbe5d7c07b3ba721ff34a8ff59fde480a4a9f
("Improves performance by lazy loading the session.")
|
|
We cannot use the sendfile(2) syscall when serving static files
to TLS clients without breaking them. We currently rely on
OpenSSL to encrypt the data before it hits the socket, so it
must be read into userspace buffers before being written to the
socket.
|
|
Using the 'update-copyright' script from gnulib[1]:
git ls-files | UPDATE_COPYRIGHT_HOLDER='all contributors' \
UPDATE_COPYRIGHT_USE_INTERVALS=2 \
xargs /path/to/gnulib/build-aux/update-copyright
We're also switching to 'GPL-3.0+' as recommended by SPDX
to be consistent with our gemspec and other metadata
(as opposed to the longer but equivalent "GPLv3 or later").
[1] git://git.savannah.gnu.org/gnulib.git
|
|
There are likely yet-to-be-discovered bugs in here.
Also, keeping explicit #freeze calls for 2.2 users, since most
users have not migrated to 2.3, yet.
|
|
We need to ensure the master process is done writing to logs
when we check them. Do that by sending a SIGQUIT to the master
to shut it down, as the master process will defer the SIGQUIT
until after the test log is written to the kernel.
|
|
Linux 4.2 gained the ability to do true zero-copy sendfile support
for Unix sockets; so buffer space is accounted differently.
Previously Linux only avoided copies in userspace when doing
sendfile for Unix sockets, not internally within the kernel.
This kernel change has no bearing on normal code which would need to
account for concurrent draining by the client; only test code
designed to create a failure condition.
|
|
Future updates may use the update-copyright script in gnulib:
git ls-files | UPDATE_COPYRIGHT_HOLDER='all contributors' \
UPDATE_COPYRIGHT_USE_INTERVALS=2 \
xargs /path/to/gnulib/build-aux/update-copyright
|
|
Process.spawn is faster under Linux since it may use vfork
to avoid marking pages copy-on-write.
|
|
Getting the logs to show up in order is tricky in a multithreaded
server...
|
|
systemd socket emulation shares FDs across execve, just like
the built-in SIGUSR2 upgrade process in unicorn. Thus it is
easy to support inheriting sockets from systemd.
|
|
We need to wait for the hijacked process to run and write to the
log before we check it. Since we may not have inotify, try to
trigger a sched_yield() syscall instead.
|
|
TCP socket options are now set when inheriting existing sockets from
a parent process. I'm fairly certain all the TCP setsockopt knobs
we use are idempotent and harmless to change.
If anything, the only directive I'd be uncomfortable changing is
shortening the listen(2) (aka :backlog) size, but we've always
changed that anyways since it also applies to UNIX sockets.
Note: removing a configuration knob in a yahns config file can not
reset the value to the OS-provided default setting. Inherited
sockets must use a new setting to override existing ones.
(or the socket needs to be closed and re-created in the process
launcher before yahns inherits it).
Based on unicorn commit 1db9a8243d42cc86d5ca4901bceb305061d0d212
Noticed-by: Christos Trochalakis <yatiohi@ideopolis.gr>
<20150626114129.GA25883@luke.ws.skroutz.gr>
|
|
We want to avoid race conditions if tests become multithreaded
from Kernel#warn internally issuing a second write.
|
|
Middlewares such as Rack::Lock (used by Rails) break badly unless
the response body is closed on hijack, so we will close it to follow
the lead of other popular Rack servers.
While it's unclear if there's anybody using rack.hijack besides
yahns/proxy_pass we'll try to emulate the behavior of other servers
as much as possible.
ref: https://github.com/ngauthier/tubesock/issues/10
While we're at it, use DieIfUsed correctly in test_ssl.rb :x
|
|
We don't require kcar to be installed since most users will never
use the proxy_pass feature, but show an informative error in case
they want to test this functionality.
|
|
Rack::TempfileReaper was added in rack 1.6 to cleanup temporary
files. Make Yahns::TmpIO ducktype-compatible and put it into
env['rack.tempfiles'] array so Rack::TempfileReaper may be used to
free up space used by temporary buffer files.
ref: commit 3bdf5481e49d76b4502c51e5bdd93f68bfd1f0b4 in unicorn
|
|
We do not want rack.hijack users relying on kgio_* methods since
kgio is trying to make itself obsolete (as Ruby itself adopts
kgio features). This is a bit wonky since our common case tries
to minimize object allocation by only using the Kgio::Socket
derived class.
|
|
The entire idea of a one-shot-based design is all the mutual
exclusion is handled by the event dispatch mechanism (epoll or
kqueue) without burdening the user with extra locking. However, the
way the hijack works means we check the Rack env for the
'rack.hijack_io' key which is shared across requests and may
be cleared.
Ideally, this would not be a problem if the Rack dispatch allowed
returning a special value (e.g. ":ignore") instead of the normal
status-headers-body array, much like what the non-standard
"async.callback" API Thin started.
We could also avoid this problem by disallowing our "unhijack-ing"
of the socket but at a significant cost of crippling code
reusability, including that of existing middleware.
Thus, we allocate a new, empty request object here to avoid a TOCTTOU
in the following timeline:
original thread: | another thread
HttpClient#yahns_step |
r = k.app.call(env = @hs.env) # socket hijacked into epoll queue
<thread is scheduled away> | epoll_wait readiness
| ReqRes#yahns_step
| proxy dispatch ...
| proxy_busy_mod_done
************************** DANGER BELOW ********************************
| HttpClient#yahns_step
| # clears env
# sees empty env: |
return :ignore if env.include?('rack.hijack_io') |
In other words, we cannot ever touch the original env seen by the
original thread since it must see the 'rack.hijack_io' value because
both are operating in the same Yahns::HttpClient object. This will
happen regardless of GVL existence.
Avoiding errors like this is absolutely critical to every one-shot
based design.
|
|
Upstreams may shut us down while we're writing a request body,
attempt to forward any responses from the upstream back to the
client which may explain the rejection reason for giant uploads.
|
|
Not all backends are capable of generating chunked responses
(especially not to HTTP/1.0 clients) nor can they generate
the Content-Length (especially when gzipping), so they'll
close the socket to signal EOF instead.
|
|
Since yahns/proxy_pass is not a drop-in replacement, reinstate
the old, synchronous version to avoid breaking existing setups
which require Rack middleware support.
|
|
kgio_writev returns nil on success instead of the number of bytes
written, so we must manually calculate the number of bytes written
intead :x
This is triggerable when sending giant chunked responses.
|
|
On one of my ancient, cache-starved home systems, this tends to
cause pathologically bad performance during some of these tests.
|