Object GET requests with a truthy X-Newest header are not resumed if a
backend request times out. The GetOrHeadHandler therefore uses the
regular node_timeout when waiting for a backend connection response,
rather than the possibly shorter recoverable_node_timeout. However,
previously while reading data from a backend response the
recoverable_node_timeout would still be used with X-Newest requests.
This patch simplifies GetOrHeadHandler to never use
recoverable_node_timeout when X-Newest is truthy.
Change-Id: I326278ecb21465f519b281c9f6c2dedbcbb5ff14
Both GetOrHeadHandler (used for replicated policy GETs) and
ECFragGetter (used for EC policy GETs) have _get_next_response_part
methods that are very similar. This patch replaces them with a single
method in the common GetterBase superclass.
Both classes are modified to use *only* the Request instance passed to
their constructors. Previously their entry methods
(GetOrHeadHandler.get_working_response and
ECFragGetter.response_parts_iter) accepted a Request instance as an
arg and the class then variably referred to that or the Request
instance passed to the constructor. Both instances must be the same
and it is therefore safer to only allow the Request to be passed to
the constructor.
The 'newest' keyword arg is dropped from the GetOrHeadHandler
constructor because it is never used.
This refactoring patch makes no intentional behavioral changes, apart
from the text of some error log messages which have been changed to
differentiate replicated object GETs from EC fragment GETs.
Change-Id: I148e158ab046929d188289796abfbbce97dc8d90
If the proxy timed out while reading a replicated policy multi-part
response body, it would transform the ChunkReadTimeout to a
StopIteration. This masks the fact that the backend read has
terminated unexpectedly. The document_iters_to_multipart_byteranges
would complete iterating over parts and send a multipart terminator
line, even though no parts may have been sent.
This patch removes the conversion of ChunkReadTmeout to StopIteration.
The ChunkReadTimeout that is now raised prevents the
document_iters_to_multipart_byteranges 'for' loop completing and
therefore stops the multi-part terminator line being sent. It is
raised from the GetOrHeadHandler similar to other scenarios that raise
ChunkReadTimeouts while the resp body is being read.
A ChunkReadTimeout exception handler is removed in the
_iter_parts_from_response method. This handler was previously never
reached (because StopIteration rather than ChunkReadTimeout was raised
from _get_next_response_part), but if it were reached (i.e. with this
change) then it would repeat logging of the error and repeat
incrementing the node's error counter.
This change in the GetOrHeadHandler mimics a similar change in the
ECFragGetter [1].
[1] Related-Chage: I0654815543be3df059eb2875d9b3669dbd97f5b4
Co-Authored-By: Tim Burke <tim.burke@gmail.com>
Change-Id: I6dd53e239f5e7eefcf1c74229a19b1df1c989b4a
The proxy should NOT read or write to memcache when handling a
container GET that explicitly requests 'shard' or 'object' record
type. A request for 'shard' record type may specify 'namespace'
format, but this request is unrelated to container listings or object
updates and passes directly to the backend.
This patch also removes unnecessary JSON serialisation and
de-serialisation of namespaces within the proxy GET path when a
sharded object listing is being built. The final response body will
contain a list of objects so there is no need to write intermediate
response bodies with a list of namespaces.
Requests that explicitly specify record type of 'shard' will of
course still have the response body with serialised shard dicts that
is returned from the backend.
Change-Id: Id79c156432350c11c52a4004d69b85e9eb904ca6
With the Related-Change, container servers can return a list Namespace
objects in response to a GET request. This patch modifies the proxy
to take advantage of this when fetching namespaces. Specifically,
the proxy only needs Namespaces when caching 'updating' or 'listing'
shard range metadata.
In order to allow upgrades to clusters we can't just send
'X-Backend-Record-Type = namespace', as old container servers won't
know how to respond. Instead, proxies send a new header
'X-Backend-Record-Shard-Format = namespace' along with the existing
'X-Backend-Record-Type = shard' header. Newer container servers will
return namespaces, old container servers continue to return full
shard ranges and they are parsed as Namespaces by the new proxy.
This patch refactors _get_from_shards to clarify that it does not
require ShardRange objects. The method is now passed a list of
namespaces, which is parsed from the response body before the method
is called. Some unit tests are also refactored to be more realistic
when mocking _get_from_shards.
Also refactor the test_container tests to better test shard-range and
namespace responses from legacy and modern container servers.
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Co-Authored-By: Jianjian Huo <jhuo@nvidia.com>
Related-Change: If152942c168d127de13e11e8da00a5760de5ae0d
Change-Id: I7169fb767525753554a40e28b8c8c2e265d08ecd
...and rename to _get_updating_shard_ranges.
The method is only used by the ObjectController, and it is only used
to fetch shard ranges in the 'updating' states.
Also relocate the associated unit tests.
Change-Id: I083e0c6898bf93d8a0dc593acd9723827e55508e
Create new helper functions to set and get namespaces in cache. Use
these in both the object and container controllers when caching
namespaces for updating and listing state shard ranges respectively.
Add unit tests for the new helper functions.
No intentional behavioural changes.
Change-Id: I6833ec64540fa19f658f0ee78952ecb43b49f169
It adds another layer of indirection and state for the sake of labeling;
longer term it'll be easier to be explicit at the point of emission.
Related-Change: I0522b1953722ca96021a0002cf93432b973ce626
Change-Id: Ieebafb19c3fa60334aff2914ab1ae70b8f140342
If the proxy-server gets an error (e.g. timeout) while reading data
from a backend node during an object GET then it will increment the
node's error limiting counter. If the node's error counter reaches a
threshold then the node is 'error-limited' i.e. not used for a period
of time.
When a read from a node fails the proxy will also attempt to resume
the read by searching for another useful node to replace the failed
node. Previously, the outcome of this search for a replacement node
erroneously determined whether the failed node's error counter was
incremented or not: the error counter was only incremented if a useful
replacement node was found.
For example, if a read from the last useful primary node failed, and
all handoff nodes then returned 404, the last primary node's would not
have its error counter incremented (for no good reason). However, if a
useful replacement handoff node was found, then the last primary
node's error counter would be incremented.
This patch fixes the inconsistent and confusing handling of node
errors by ensuring that the node error limiting counter is *always*
incremented when reading from a node fails, *before* the search for a
replacement node.
Note: The Related-Change fixed a bug whereby the replacement node's
error counter was incremented rather than the failed node's error
counter. This patch addresses the inconsistent incrementing of the
failed node's error counter.
Related-Change: Ibd405b79820cedaf9cb02906b8c13c8d34f4d7ec
Change-Id: I938ff7ed7bcbfb363f55821cccd92763dfce449d
Fix an AttributeError that may be raised during exception handling in
the proxy object GET path. GetterBase.node was removed by the
RelatedChange.
Change-Id: Icf77b9d6b8065814033059e33aca6e98a633dbd4
RelatedChange: I76ea042ef4b3f5cc1caa4774e35d6191f4ca548e
Both the EC and replicated GET paths have is_good_source() methods
that are similar. Refactor to have just one implementation.
Change-Id: I661d3704a76e3d92bfcfeaed1fff4ed5e28c79b4
This patch tries to make the proxy-server GET path a little easier to
read. The changes are primarily to the GetOrHeadHandler and
ECFragGetter classes.
- Make generator method names more uniform.
- Use _<method> to make it more obvious which methods are internal
to the class and which methods are the interface to the class.
- Move the interface method to the end of the class.
- Add some commentary and docstrings.
No intended behavioral change.
Change-Id: I3d00b1071669a42526a31588a2643f91c58ea5a8
The proxy GetterBase manages a set of attributes related to the
backend source for a GET response: source (a response object), node
and source_parts_iter. These attributes are always updated together so
benefit from encapsulation, along with some helper methods to simplify
the GET paths.
Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
Change-Id: I76ea042ef4b3f5cc1caa4774e35d6191f4ca548e
This patch will add more granularity to metrics of account_info or
container_info cache and related backend lookups.
Before this patch, related metrics are:
1.account.info.cache.[hit|miss|skip]
2.container.info.cache.[hit|miss|skip]
With this patch, they are going to become:
1.account/container.info.infocache.hit
cache hits with infocache.
2.account/container.info.cache.hit
cache hits with memcache.
3.account/container.info.cache.[miss|skip|disabled]
.<status_int>
Those are operations made to backend due to below reasons.
miss: cache misses.
skip: the selective skips per skip percentage config.
disabled: memcache is disabled.
For each kind of operation metrics, suffix <status_int> will
count operations with different status. Then a sum of all
status sub-metrics will the total metrics of that operation.
UpgradeImpact
=============
Metrics dashboard will need updates to display those changed metrics
correctly, also some infocache metrics are newly added, please see
above message for all changes needed.
Change-Id: I60a9f1c349b4bc78ecb850fb26ae56eb20fa39c6
A while back, we changed get_account_info and get_container_info to
call the proxy-server app directly, rather than whatever was right
of the current middleware. This reduced backend request amplification
on cache misses.
However, it *also* meant that we stopped emitting logs or metrics in
the proxy for these backend requests. This was an unfortunate and
unintended break from earlier behavior.
Now, extend the middleware decorating we're doing in loadapp() to
include a "logged app", i.e., the app wrapped by it's right-most
proxy-logging middleware. If there is not logging middleware (such
as would happen for the backend servers), the "logged app" will be
the main app. Make account and container info requests through
*that* app, so we get logging and metrics again.
Closes-Bug: #2027726
Related-Change: I49447c62abf9375541f396f984c91e128b8a05d5
Change-Id: I3f531f904340e4c8407185ed64b41d7d614a308a
The client_chunk_size attribute was introduced into GetOrHeadHandler
for EC support [1]. It was only ever not None for an
ECObjectController. The ECObjectController stopped using
GetOrHeadHandler for Object GET when the ECFragGetter class was
introduced [2], but the EC specific code was not expunged from
GetOrHeadHandler. In [3] the ECFragGetter client_chunk_size was renamed
to fragment_size to better reflect what it represented.
The skip_bytes attribute was similarly introduced for EC support. It
is only ever non-zero if client_chunk_size is an int. For EC,
skip_bytes is used to undo the effect of expanding the backend
range(s) to fetch whole fragments: the range(s) of decoded bytes
returned to the client may need to be narrower than the backend
ranges. There is no equivalent requirement for replicated GETs.
The elimination of client_chunk_size and skip_bytes simplifies the
yielding of chunks from the GetOrHeadHandler response iter.
Related-Change:
[1] I9c13c03616489f8eab7dcd7c5f21237ed4cb6fd2
[2] I0dc5644a84ededee753e449e053e6b1786fdcf32
[3] Ie1efaab3bd0510275d534b5c023cb73c98bec90d
Change-Id: I31ed36d32682469e3c5ca8bf9a2b383568d63c72
The proxy has a class of "recoverable" timeouts; when they trip, it
tries to resume from another node, with a range offset that accounts
for the bytes already served.
If we set the X-Backend-Ignore-Range-If-Metadata-Present header (like
we do in SLO), it may cause the object servers to return the *whole*
object. When that happens on the initial GET, the proxy is smart enough
to still track the appropriate range to fetch when resuming -- but then
it would send the ignore-range header so the new object-server would
send the whole object.
This would cause problems in one of two ways:
For replicated policies, too many bytes would be yielded out of
the proxy app. proxy-logging would notice, truncate the response,
and log a warning about it.
For EC policies, at least one frag iter would have the wrong
length, causing a decode error from pyeclib.
Now, pop off that header, if present.
Change-Id: I31f622f6ebfa255d4c52089ca182cbf665c40a30
In some cases, when resuming a get using a new source node, the new
source node would be error limited rather than the old node. One case
was previously fixed [1], but other erroneous cases remain.
This patch adds a helper method to unify handling of all cases when an
old node should be error limited, and ensures that the old rather than
new node is limited.
[1] Related-Change: I02f9c0ffd879423211b87f7ac5acde762933f47d
Change-Id: Ibd405b79820cedaf9cb02906b8c13c8d34f4d7ec
The GetOrHeadHandler class in base.py and the ECFragGetter
class in obj.py have some methods that are very similar.
The two existing classes do also have unique code so they
will remain, but as subclasses of the common base class
called GetterBase.
Change-Id: I893c5fcb6b4f8a7dda351169f5f6b37375a34817
Previously it was possible for an entire object PUT data transfer to
execute without the greenthread sleeping and allowing other
greenthreads to run. This was more likely with an EC PUT because the
computation of EC fragments might be slower than the rate at which
they are drained out of IO send buffers, so IO never blocks. In
extreme cases this could cause timeouts in other greenthreads to pop.
This patch adds a periodic zero-time sleep in the object PUT data
transfer loop. An existing pattern in the GET path is re-used, and
extracted to a new CooperativeIterator helper class.
Change-Id: Idd6b767f1a746c72c106199f5d1fada3615b1e97
Closes-Bug: #2019955
Related-Change: Iae27109f5a3d109ad21ec9a972e39f22150f6dbb
X-Backend-* headers were previously passed to the backend server with
only a subset of all request types:
* all object requests
* container GET, HEAD
* account GET, HEAD
In these cases, X-Backend-* headers were transferred to backend
requests implicitly as a consequence of *all* the headers in the
request that the proxy is handling being copied to the backend
request.
With this change, X-Backend-* headers are explicitly copied from the
request that the proxy is handling to the backend request, for every
request type.
Note: X-Backend-* headers are typically added to a request by the
proxy app or middleware, prior to creating a backend request.
X-Backend-* headers are removed from client requests by the gatekeeper
middleware, so clients cannot send X-Backend-* headers to backend
servers. An exception is an InternalClient that does not have
gatekeeper middleware, deliberately so that internal daemons such as
the sharder can send X-Backend-* headers to the backend servers.
Also, BaseController.generate_request_headers() is fixed to prevent
accessing a None type when transfer is True but the orig_req is None.
Change-Id: I05fb9a3e1c98d96bbe01da2ee28474e0f57297e6
Updating shard range cache has been restructured and upgraded to v2
which only persist the essential attributes in memcache (see
Related-Change). This is the following patch to restructure the
listing shard ranges cache for object listing in the same way.
UpgradeImpact
=============
The cache key for listing shard ranges in memcached is renamed
from 'shard-listing/<account>/<container>' to
'shard-listing-v2/<account>/<container>', and cache data is
changed to be a list of [lower bound, name]. As a result, this
will invalidate all existing listing shard ranges stored in the
memcache cluster.
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Related-Change: If98af569f99aa1ac79b9485ce9028fdd8d22576b
Change-Id: I54a32fd16e3d02b00c18b769c6f675bae3ba8e01
Restructure the shard ranges that are stored in memcache for
object updating to only persist the essential attributes of
shard ranges in memcache (lower bounds and names), so the
aggregate of memcache values is much smaller and retrieval
will be much faster too.
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Co-Authored-By: Tim Burke <tim.burke@gmail.com>
UpgradeImpact
=============
The cache key for updating shard ranges in memcached is renamed
from 'shard-updating/<account>/<container>' to
'shard-updating-v2/<account>/<container>', and cache data is
changed to be a list of [lower bound, name]. As a result, this
will invalid all existing updating shard ranges stored in the
memcache cluster.
Change-Id: If98af569f99aa1ac79b9485ce9028fdd8d22576b
Following a memcache restart in a SAIO, I've seen the following happen
during an object HEAD:
- etag_quoter wants to get account/container info to decide whether to
quote-wrap or not
- account info is a cache miss, so we make a no-auth'ed HEAD to the next
filter in the pipeline
- eventually this gets down to ratelimit, which *also* wants to get
account info
- still a cache miss, so we make a *separate* HEAD that eventually talks
to the backend and populates cache
- ratelimit realizes it can't ratelimit the request and lets the
original HEAD through to the backend
There's a related bug about how something similar can happen when the
backend gets overloaded, but *everything is working* -- we just ought to
be talking straight to the proxy app.
Note that there's likely something similar going on with container info,
but the hardcoded 10% sampling rate makes it harder to see if you're
monitoring raw metric streams.
I thought I fixed this in the related change, but no :-/
Change-Id: I49447c62abf9375541f396f984c91e128b8a05d5
Related-Change: If9249a42b30e2a2e7c4b0b91f947f24bf891b86f
Related-Bug: #1883214
Adding a "use_replication" field to the node dict, a helper function to
set use_replication dict value for a node copy by looking up the header
value for x-backend-use-replication-network
Change-Id: Ie05af464765dc10cf585be851f462033fc6bdec7
Function to get update shards is only related to ObjectController
in obj.py, similar to existing get list shards function of
ContainerController in container.py.
Change-Id: Ie20fbf9b46db20f2928198a16305d7509af833db
This patch will add more granularity to shard operation cache or
backend metrics, and then remove some of existing and duplicated
metrics.
Before this patch, related metrics are:
1.shard_<op>.cache.[hit|miss|skip]
2.shard_<op>.backend.<status_int>
where op is 'listing' or 'updating'.
With this patch, they are going to become:
1.shard_<op>.infocache.hit
cache hits with infocache.
2.shard_<op>.cache.hit
cache hits with memcache.
3.shard_<op>.cache.[miss|bypass|skip|force_skip|disabled|error]
.<status_int>
Those are operations made to backend due to below reasons.
miss: cache misses.
bypass: metadata didn't support a cache lookup
skip: the selective skips per skip percentage config.
force_skip: the request with 'x-newest' header.
disabled: memcache is disabled.
error: memcache connection error.
For each kind of operation metrics, suffix <status_int> will
count operations with different status. Then a sum of all
status sub-metrics will the total metrics of that operation.
UpgradeImpact
=============
Metrics dashboard will need updates to display those changed metrics
correctly, also infocache metrics are newly added, please see above
message for all changes needed.
Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
Change-Id: Ib8be30d3969b4b4808664c43e94db53d10e6ef4c
This patch introduces a check_response method to proxy server
Application class. The method provides common checking and reporting
of backend response errors (i.e. responses with status code >=
500). This checking was previously duplicated in proxy controller
classes.
The formatting and content of error logging messages is now more
unform, which does result in changes to some logged error messages.
Change-Id: I4b935a47f39642c11364f2a8d50a5f2ebb3b84f1
If you've got thousands of requests per second for objects in a single
container, you basically NEVER want that container's info to ever fall
out of memcache. If it *does*, all those clients are almost certainly
going to overload the container.
Avoid this by allowing some small fraction of requests to bypass and
refresh the cache, pushing out the TTL as long as there continue to be
requests to the container. The likelihood of skipping the cache is
configurable, similar to what we did for shard range sets.
Change-Id: If9249a42b30e2a2e7c4b0b91f947f24bf891b86f
Closes-Bug: #1883324
Alistair Coles