If a client were in the middle of an object GET request and then
disconnected, the proxy would wait a while (default 60s) and then time
out the connection. As part of the teardown for this, the proxy would
attempt to close the connection to the object server, then drain any
associated buffers. However, this didn't work particularly well,
resulting in the proxy reading the entire remainder of the object for
no gain.
Now, the proxy closes the connection hard, by calling .close() on the
underlying socket._socket object. This is different from calling
.close() on a socket._socketobject object, which is what you get back
from socket.socket() and similar methods. Calling .close() on a
socket._socketobject simply decrements a reference counter on the
socket._socket, which has been observed in the past to result in
socket leaks when something holds onto a reference. However, calling
.close() on a socket._socket actually closes the socket regardless of
who else has a reference to it.
I had to delete a test assertion that said the object server never got
SIGPIPE after a GET w/X-Newest. Well, you get a SIGPIPE when you write
to a closed socket, and now the proxy is actually closing the sockets
early, so now you *do* get a SIGPIPE.
closes-bug: 1174660
Note that this will cause a regression on bug 1037337; unfortunately,
the cure is worse than the disease, so out it goes.
Change-Id: I9c7a2e7fdb8b4232e53ea96f86b50e8d34c27221
Fixes object versioning when object name and/or version
container name contain multibyte utf-8 characters.
When object names containing non-ASCII characters
are PUT multiple times into a container with an
x-versions-location set, subsequent DELETE of the
object results in a 500 response status.
When the versions container name contains
non-ASCII characters the first delete of an object
succeeds but fails to restore previous version of
object, so second delete incorrectly returns 404.
Fixes bug 1229142
Change-Id: I425440f76b8328f8e119d390bfa4c7022181e89e
Place all the methods related to on-disk layout and / or configuration
into a new common module that can be shared by the various modules
using the same on-disk layout.
Change-Id: I27ffd4665d5115ffdde649c48a4d18e12017e6a9
Signed-off-by: Peter Portante <peter.portante@redhat.com>
I knew webob.Request.blank could take most of the attributes on the class as
kwargs to blank, so I went and looked how. It seems to work ok and is pretty
nice.
Change-Id: I72fae7c28f81c97768ee98b8ebcd69789a4c2e84
Since we've got machinery to catch HTTPException errors that happens
after the proxy server's __call__ method but before the individual
GET()/POST()/etc. methods, we shouldn't just call
GET()/POST()/etc. Instead, we should use swob.Request's get_response()
method, or otherwise actually call the proxy application. That way we
can have the code raise HTTPBadRequest, but the test just gets a
plain old response with a 400 status code, just like you'd expect.
Earlier changes fixed some of the tests; this commit fixes some
more. In the interest of not having a gigantic diff, this only gets
about half the proxy's unit tests, and the others will be done in a
subsequent commit.
Change-Id: Iaca32e74a8f4499440e655c207a1012e14486c74
There were a few different places where we had some repeated code to
figure out what format an account or container listing response should
be in (text, JSON, or XML). Now that's been pulled into a single
function.
As part of this, you can now raise HTTPException subclasses in proxy
controllers instead of laboriously plumbing error responses all the
way back up to swift.proxy.server.Application.handle_request(). This
lets us avoid certain ugly patterns, like the one where a method
returns a tuple of (x, y, z, error) and the caller has to see if it
got (value, value, value, None) or (None, None, None, errorvalue). Now
we can just raise the error.
Change-Id: I316873df289160d526487ad116f6fbb9a575e3de
On PyPy adding __del__ to a class after it's constructed does not
work, instead implement the leakchecker in tests by using weakrefs,
which works everywhere.
Change-Id: I873efb3e2f85f731d5836bf9bf06a21e939e8542
An object GET request with both If-None-Match: <Etag> and
Range: bytes=N-M should result in a 304 Not Modified if the Etag
matches. However, it was resulting in a 416 Requested Range Not
Satisfiable. This commit fixes that.
The fix was to remove conditional_response=True from a request in the
proxy. As far as I can see, the only effect that
conditional_response=True has in swob is to enable range
checking. Since the object server handles all the range stuff, the
proxy shouldn't do it. Otherwise, you wind up with double-application
of the Range header, and that doesn't work very well.
This is probably left over from the WebOb days.
Fixes bug #1196805.
Change-Id: I207f3839731b7503baa3fdbd7cd1cfe3e1b46f62
Be able to tell swift to figure out the content-type even if it is
sent because old client code / curl has trouble sending blank
content-type headers.
Change-Id: Ie65ddf8993a19ea74e0b85a2ae56da84a617c19d
A failure to close files in a timely fassion means
that data is not necessarily written immediately
on Pythons which do not use reference counting
(e.g. PyPy).
Change-Id: I5d363249676032a025a22a67275c2eed3151b264
test_DELETE_x_container_headers_with_more_container_replicas() would
sometimes fail, and it's because a test helper method was defaulting
to sorting the captured headers by X-Container-Partition. Well, in
tests we use FakeRing, and the partition is always 1, so that sorting
didn't buy us much. Now the sorting is done by X-Container-Device,
which actually differs meaningfully.
Change-Id: Ibe5b2fcd3f23280ed2caaa703111a98861331866
We've gone back and forth about this. In the initial commit, it couldn't
possibly work because you wouldn't be able to get the Etags to match. Then it
was expressly disallowed with a custom error message, and now its allowed. The
reason we're allowing it is that 1,000 segments isn't enough for some use cases
and we decided its better than just upping the number of allowed segments. The
code to make it work isn't all that complicated and it allows for virtually
unlimited SLO object size. There is also a new configurable limit on the
maximum connection time for both SLOs and DLOs defaulting to 1 day. This will
hopefully alleviate worries about infinite requests. Think I'll leave the
python-swift client support for nested SLOs to somebody else though :).
DocImpact
Change-Id: Id16187481b37e716d2bd09bdbab8cc87537e3ddd
Currently clients can not distinguish between non existing accounts
(which can be created) and accounts marked for deletion, which has
not yet been reaped and therefore cannot be re-created until reaped.
Following this patch, if an account is marked as deleted but hasn't
been reaped and is still on disk, responses will include a status
header:
'X-Account-Status' = 'Deleted'
Fixes:Bug #1188609
Change-Id: Ibd39965ae3f5d45fd78f130e0e31f5a0141a8633
The proxy can now be configured to prefer local object servers for PUT
requests, where "local" is governed by the "write_affinity". The
"write_affinity_node_count" setting controls how many local object
servers to try before giving up and going on to remote ones.
I chose to simply re-order the object servers instead of filtering out
nonlocal ones so that, if all of the local ones are down, clients can
still get successful responses (just slower).
The goal is to trade availability for throughput. By writing to local
object servers across fast LAN links, clients get better throughput
than if the object servers were far away over slow WAN links. The
downside, of course, is that data availability (not durability) may
suffer when drives fail.
The default configuration has no write affinity in it, so the default
behavior is unchanged.
Added some words about these settings to the admin guide.
DocImpact
Change-Id: I09a0bd00524544ff627a3bccdcdc48f40720a86e
If you make a range request for a manifest object, then the proxy can
retrieve more data than it actually needs from the object server. It
sends the right bytes to the client, and then spends a bunch of time
reading and discarding bytes it doesn't need.
Example:
file start file end
| range start range end |
v v v v
[--------------------------------------------------]
The proxy would send a request to the object server with "Range:
bytes=${range start}-", and so the object server responds with all the
bytes from range start to file end. This needlessly transfers all the
bytes from range end to file end, as none of those are sent to the
client.
This commit fixes the sent Range header to only request the bytes
needed.
Change-Id: Ie8d4ee2c5e67c08795ac2d39d1df7d11856621a9
Now you can configure the proxy server to read from "local" primary
nodes first, where "local" is governed by the newly-introduced
"read_affinity" setting in the proxy config. This is desirable when
the network links between regions/zones are of varying capacities; in
such a case, it's a good idea to prefer fetching data from closer
backends.
The new setting looks like rN[zM]=P, where N is the region number, M
is the optional zone number, and P is the priority. Multiple values
can be specified by separating them with commas. The priority for
nodes that don't match anything is a very large number, so they'll
sort last.
This only affects the ordering of the primary nodes; it doesn't affect
handoffs at all. Further, while the primary nodes are reordered for
all requests, it only matters for GET/HEAD requests since handling the
other verbs ends up making concurrent requests to *all* the primary
nodes, so ordering is irrelevant.
Note that the default proxy config does not have this setting turned
on, so the default configuration's behavior is unaffected.
blueprint multi-region
Change-Id: Iea4cd367ed37fe5ee69b63234541d358d29963a4
See Bug 1187200 for a full description of the problem.
Part 1:
X-Delete-At-Container added to X-Delete-At-* info
This fixes the bug by passing the expiring-objects-account's
container name onward to the backend object servers. This is in case
the object servers' expiring_objects_container_divisor happens to be
different than the proxy server's, we want to make sure the host,
partition, and device match up with the container name. Different
container names would be fine, but not with mismatched host,
partition, and device info.
Part 2:
The db_replicator now double checks the disk path's partition against
the partition the ring gives back. If they don't match, it logs the
problem but continues to replicate the database to where it should be
and, on success to all proper nodes, removes the local out of place
database.
Bug 1187200
Change-Id: Id0873a3f2198ce285fe0b0c777738eff38bc2438
Consolidate the different ways in which info of account/container
is gathered, cached, used, updated, etc.
This refactoring increases code reuse and is a basis for later
addition of account ACLs.
Changing the get_info users is left for future.
This staged approach ensures the behaviour is unchanged.
Change-Id: I67b58030d3f9e3bc86bcd7ece0f1dc693c4e08c3
Fixes: Bug #1162199
If account autocreation is on and the proxy receives a GET request for
a nonexistent account, it'll fake up a response that makes it look as
if the account exists, but without reifying that account into sqlite
DB files.
That faked-out response was just fine as long as you wanted a
text/plain response, but it didn't handle the case of format=json or
format=xml; in those cases, the response would still be
text/plain. This can break clients, and certainly causes crashes in
swift3. Now, those responses match as closely as possible.
The code for generating an account-listing response has been pulled
into (the new) swift.account.utils module, and both the fake response
and the real response use it, thus ensuring that they can't
accidentally diverge. There's also a new probe test for that
non-divergence.
Also, cleaned up a redundant matching of the Accept header in the code
for generating the account listing.
Note that some of the added tests here pass with or without this code
change; they were added because the code I was changing (parts of the
real account GET) wasn't covered by tests.
Bug 1183169
Change-Id: I2a3b8e5d9053e4d0280a320f31efa7c90c94bb06
Our tests get rather grabby with file descriptors, mostly due to the
SysLogHandler. In recent work I'm doing, my additional tests put
things just over the 1024 limit and tests would start to fail
somewhat randomly. This alleviates the problem by mocking out one of
the bigger users of SysLogHandler, proxy/test_server.py. If you want
to verify this has an impact, you can put the following in your
proxy/test_server.py's overall teardown method and try it with and
without the mocking:
import subprocess
print >>sys.stderr, '%s OPEN FILES' % len(subprocess.Popen(
['lsof'], stdout=subprocess.PIPE).communicate()[0].split('\n'))
Change-Id: I1bd3efe46ee69d09a32c5a964f04e36e46506446
RFC 2616 Sec 4.4 Message Length describes how the content-length and
transfer-encoding headers interact. Basically, if chunked transfer
encoding is used, the content-length header value is ignored and if
the content-length header is present, and the request is not using
chunked transfer-encoding, then the content-length must match the body
length.
The only Transfer-Coding value we support in the Transfer-Encoding
header (to date) is "chunked". RFC 2616 Sec 14.41 specifies that if
"multiple encodings have been applied to an entity, the
transfer-codings MUST be listed in the order in which they were
applied." Since we only supported "chunked". If the Transfer-Encoding
header value has multiple transfer-codings, we return a 501 (Not
Implemented) (see RFC 2616 Sec 3.6) without checking if chunked is the
last one specified. Finally, if transfer-encoding is anything but
"chunked", we return a 400 (Bad Request) to the client.
This patch adds a new method, message_length, to the swob request
object which will apply an algorithm based on RFC 2616 Sec 4.4
leveraging the existing content_length property.
In addition to these changes, the proxy server will now notice when
the message length specified by the content-length header is greater
than the configured object maximum size and fail the request with a
413, "Request Entity Too Large", before reading the entire body.
This work flows from https://review.openstack.org/27152.
Change-Id: I5d2a30b89092680dee9d946e1aafd017eaaef8c0
Signed-off-by: Peter Portante <peter.portante@redhat.com>
Autocreate was messy - now cleaned.
Auto-create now occurs at account POST, and container PUT only.
A method for autocreation was added
Autocreation was removed from account_info and container_info.
Fake-it as if the account exists on account HEAD and account GET.
Return 404 on everything else when the account does not exist.
Fix: Bug #1172223
Fix: Bug #1179140
Change-Id: Iac54c1438eb09883fbc29a1ad2ac2245b95efc92
Enhance internally logged messages to report referer and user-agent.
Pass the referering URL and METHOD between internal servers (when
known), and set the user-agent to be the server type (obj-server,
container-server, proxy-server, obj-updater, obj-replicator,
container-updater, direct-client, etc.) with the process PID. In
conjunction with the transaction ID, it helps to track down which PID
from a given system was responsible for initiating the request and
what that server was working on to make this request.
This has been helpful in tracking down interactions between object,
container and account servers.
We also take things a bit further performaing a bit of refactoring to
consolidate calls to transfer_headers() now that we have a helper
method for constructing them.
Finally we performed further changes to avoid header key duplication
due to string literal header key values and the various objects
representing headers for requests and responses. See below for more
details.
====
Header Keys
There seems to be a bit of a problem with the case of the various
string literals used for header keys and the interchangable way
standard Python dictionaries, HeaderKeyDict() and HeaderEnvironProxy()
objects are used.
If one is not careful, a header object of some sort (one that does not
normalize its keys, and that is not necessarily a dictionary) can be
constructed containing header keys which differ only by the case of
their string literals. E.g.:
{ 'x-trans-id': '1234', 'X-Trans-Id': '5678' }
Such an object, when passed to http_connect() will result in an
on-the-wire header where the key values are merged together, comma
separated, that looks something like:
HTTP_X_TRANS_ID: 1234,5678
For some headers in some contexts, this is behavior is desirable. For
example, one can also use a list of tuples which enumerate the multiple
values a single header should have.
However, in almost all of the contexts used in the code base, this is
not desirable.
This behavior arises from a combination of factors:
1. Header strings are not constants and different lower-case and
title-case header strings values are used interchangably in the
code at times
It might be worth the effort to make a pass through the code to
stop using string literals and use constants instead, but there
are plusses and minuses to doing that, so this was not attempted
in this effort
2. HeaderEnvironProxy() objects report their keys in ".title()"
case, but normalize all other key references to the form
expected by the Request class's environ field
swob.Request.headers fields are HeaderEnvironProxy() objects.
3. HeaderKeyDict() objects report their keys in ".lower()" case,
and normalize all other key references to ".lower()" case
swob.Response.headers fields are HeaderKeyDict() objects.
Depending on which object is used and how it is used, one can end up
with such a mismatch.
This commit takes the following steps as a (PROPOSED) solution:
1. Change HeaderKeyDict() to normalize using ".title()" case to
match HeaderEnvironProxy()
2. Replace standard python dictionary objects with HeaderKeyDict()
objects where possible
This gives us an object that normalizes key references to avoid
fixing the code to normalize the string literals.
3. Fix up a few places to use title case string literals to match
the new defaults
Change-Id: Ied56a1df83ffac793ee85e796424d7d20f18f469
Signed-off-by: Peter Portante <peter.portante@redhat.com>
Increase testing coverage and make some of the test more accurate
(Some of the tests included too many responses from the account
servers that were not used by the test)
Change-Id: Ide2e0fcb89d5905e70d3111f7ac57b1cff23a99c
Fixes: Bug #1172152
Current codes will copy metadata headers when x-fresh-metadata:false, we
still need copy "x-delete-at" header and ensure expiring work at the same
time.
Change-Id: Ie31326b5f7b565e51e5aa249279bc1786f7bc847
Fixes: bug #1067528
I just noticed tonight when adding a bunch of stuff to Swiftly that
the Bulk Delete middleware uses an account-level DELETE request,
albeit with a query parameter of bulk-delete. But, one typo and,
assuming the cluster supports it and you have access, whoops, you
just marked the account for deletion!
I put a bit of extra safety on the account deletion by requiring it
to have an empty query string.
Change-Id: Ib5df11193b04eff69d14185bd9d0607169131e7f
