openstack/swift - swift - OpenDev: Free Software Needs Free Tools

It's confusing and unecessary to have the last cycles per-device
object-updater stats reaped from recon immediately during aggregation
and make it impossible to debug stats aggregation.
Drive-by: fix some bugs with stats aggregation
Change-Id: I9df7c2d1c31646a3200614b629598576eb9e64c0

Previously, we were relying on some xfs-specific return order.
Change-Id: If9a0fdb3749a18a9479f20fb174e0c1908a783bb

Previously, it was possible for updater_workers to be a negative integer
or zero. This change enforces that updater_workers should be a positive
integer.
Change-Id: Ie40194b406aeedcf8c38a3c273ab768e2b643a5d

Change-Id: I22674f2e887bdeeffe325efd2898fb90faa4235f

Address an issue where `OldestAsyncManager` instances created before forking resulted in each child process maintaining its own isolated copy-on-write stats, leaving the parent process with an empty/unused instance. This caused the final `dump_recon` call at the end of `run_forever` to report no meaningful telemetry.
The fix aggregates per-disk recon stats collected by each child process. This is done by loading recon cache data from all devices, consolidating key metrics, and writing the aggregated stats back to the recon cache.
Change-Id: I70a60ae280e4fccc04ff5e7df9e62b18d916421e

All we need is int(). Using eval() on user-provided data (or really at
all) is a Bad Idea.
Closes-Bug: #2091124
Change-Id: I39bb87f9d8e27f2f88410a087a120a0e9be1a243

Add some commentary as a reminder that whilst normal object updates to
the ContainerBroker cannot have content-type timestamp older than data
timestamp, expirer queue updates can.
Change-Id: I6d8ad06c645f25497dc15173460430fd93747afa
Related-Change: Ie4b25f1bd16def4069878983049b83de06f68e54

Related-Change: Iba43783e880e0860357ba8b9f0a11f28abf87555
Change-Id: I1e096dab9a97956bf786ccbcd37c20f9a3a5429e

Change-Id: I9473fad7c46ac03bbc71328c17e988af9d21386c

Change-Id: Ib8196fe24f8a999315af469a435bd639378c78a9

Our API ref says "Valid values are json, xml, or plain. The default is
plain." There's no reason our tests ought to use an invalid value; it
will confuse anyone looking at tests for how to do a thing.
Note that tests were passing because invalid values are ignored, so
?format=txt behaves exactly like ?format=plain.
Change-Id: I6e119cc9c7297d8aade9736fa1d6f4a105466d77

Currently, when object-server serves PUT request and DiskFile
writer write file chunks to disk, there is no explicit
eventlet sleep called. When network outpace the slow disk IO,
it's possible one large and slow PUT request could cause
eventlet hub not to schedule any other green threads for a
long period of time. To improve this, this patch enable the
configurable yield parameter 'cooperative_period' into object
server controller write path.
Related-Change: I80b04bad0601b6cd6caef35498f89d4ba70a4fd4
Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
Change-Id: I1c0aba9830433f093d024b4c39cd3a3b2f0d69f1

We would like to remove any Statsd related code from logs.py. That
requires that SwiftLogAdapter no longer provides a StatsdClient
interface by default. However, for backwards compatibility the main
utils.get_logger/get_prefixed_logger entrypoints must provide a
SwiftLogAdapter that does have a StatdsClient interface.
The new utils._patch_statsd_methods helper function is therefore used
to retrospectively patch a SwiftLogAdapter instance with the
StatsdClient interface when necessary. The _patch_statsd_helper is
used in get_logger, and again when we clone a logger in
get_prefixed_logger.
Co-Authored-By: Shreeya Deshpande <shreeyad@nvidia.com>
Change-Id: I44694b92264066ca427bb96456d6f944e09b31c0

Change-Id: I5c9af73cc66e17dc662d8c4d97cbf71ded1fed6f

A failing CORS test in the gate discovered that, when running with
eventlet==0.38.0, container and account HEAD requests returned
Content-Type of application/json to clients regardless of the requested
format. This was due to the backend HEAD response no longer having a
Content-Length header, causing the listing_formats middleware to not
modify the returned Content-Type (see Related-Change).
The Related-Change fixed the client facing issue by making
listing_formats middleware ensure the correct Content-Type is returned
to clients even when Content-Length is absent in the backend
response. The Related-Change also ensured that the 204 response to
clients always has a Content-Length header.
This patch directly fixes the problem of backend account and container
server HEADs no longer having 'Content-Length: 0' by adding it
explicitly. This violates the RFC prohibition of a 204 response having
a Content-Length header [1], but preserves Swift's historic behavior
and is consistent with the proxy-server's 204 response to clients.
[1] https://httpwg.org/specs/rfc7230.html#header.content-length
Related-Change: If724485e1425d1481d10b9255436301e346f07e8
Change-Id: Idacc59c5f43367926eff5221ee7fc417a9bc2d50

A failing CORS test in the gate discovered that we were responding
application/json to ?format=txt requests (which is maybe not even a
valid value for that qs param?), but only when running with
eventlet==0.38.0
This avoids the problem of backend container server HEADs no longer
having 'Content-Length: 0' by fixing the client HEAD resp headers before
we check for chunked-transfer resp.
Drive-By: refactor listing_formats to use HeaderKeyDict and always set
Content-Length explicitly
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Co-Authored-By: Matthew Oliver <matt@oliver.net.au>
Change-Id: If724485e1425d1481d10b9255436301e346f07e8

This patch fixes a bug in the Related-Change that causes a prefix to
be added twice at the start of an 'exception' log message.
Previously, PrefixLoggerAdapter.exception would forward to the wrapped
LogAdapter.exception, and not call back to
PrefixLoggerAdapter.process. The prefix therefore needed to be added
in both PrefixLoggerAdapter.exception and PrefixLoggerAdapter.process
(for other log level methods).
SwiftLogAdapter.exception *does* call back to SwiftLogAdapter.process,
and so since the Related-Change it is not necessary to add the prefix
in both the SwiftLogAdapter.exception and SwiftLogAdapter.process
methods.
Related-Change: I8988c0add6bb4a65cc8be38f0bf527f141aac48a
Change-Id: Ia6e1f007989b0ef455b8dca8155b386a3fd9e8e1

In order to modernize swift's statsd configuration we're working to
separate it from logging. This change is a pre-requisite for the
Related-Change in order to simplfy the stdlib base logger instance
wrapping in a single extended SwiftLogAdapter (previously LogAdapter)
which supports all the features swift's servers/daemons need
from our logger instance interface.
Related-Change-Id: I44694b92264066ca427bb96456d6f944e09b31c0
Change-Id: I8988c0add6bb4a65cc8be38f0bf527f141aac48a

Previously, queue-clean-up would use the same timestamp/offset as the
tombstone written down in the old policy to clean up now-moved data.
This could lead to bad behaviors, as the reconciler-written tombstone
could itself be enqueued to be reconciled, at the same timestamp. If
that happened, replication would never bring the DB replicas to
consistency, causing the reconciler to get different answers for
whether there was work to do.
Now, add an extra offset bump between the tombstone in the old policy
and queue-clean-up. Also add an extra offset to the moved data in the
new policy, to keep it one ahead of queue-clean-up. New ordering looks
like:
- data/ts in old policy at t0 (as well as queue entry time to move it)
- tombstone in old policy at t0_1
- queue clean-up time at t0_2
- moved data/ts in new policy at t0_3
Closes-Bug: #2028175
Change-Id: Ib0dda0d338f48336d18d3d817a0c5994e201042e

In the past we have had some async pendings that repeatedly fail for months at a time. This patch adds an OldestAsyncPendingTracker class which manages the tracking of the oldest async pending updates for each account-container pair. This class maintains timestamps for pending updates associated with account-container pairs. It evicts the newest pairs when the max_entries is reached. It supports retrieving the N oldest pending updates or calculating the age of the oldest pending update.
Change-Id: I6d9667d555836cfceda52708a57a1d29ebd1a80b

This commit differentiates async pendings that are unlinked due to a successful object update (unlinks)from those async pendings that are unlinked due to a newer async_pending existing for the same object (outdated_unlinks).
Change-Id: I66e16207f3e368248617fc7ed3c6b5c80c54b1b5

Drive-Bys:
 * DRY out redundent configuration examples in expiring objects overview
 documentation.
 * Add missing delay_reaping man page docs.
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Change-Id: I8879dbd13527233c878dff764ec411ce9619ee39

This change enhances the FakeInteralClient to make it easier to use the
existing stubs to test error handling behaviors and make assertions
about the behaviors.
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Change-Id: Ib652a752ba91dbc791eef2a1d6940bcb0d16e36a

Fixes a bug with the x-open-expired feature where our magic header
does not get copied when refetching all manifests that causes
404 on HEAD requests with part-number=N query parameter since
the object-server returns an empty response body and the
proxy needs to refetch. The fix also applies to segment GET
requests if the segments have expired.
Change-Id: If0382d433f73cc0333bb4d0319fe1487b7783e4c

Previously FakeSwift would raise a KeyError for a call that had no
matching registered response *before* capturing the call. This
prevents tests reliably asserting that only expected calls have been
made. In particular, if the code being tested handles the KeyError
gracefully then it was possible to write quite reasonable test
assertions that passed despite unexpected calls being made.
Tests in test/unit/container/test_reconciler.py seem to rely on this
behaviour, so this patch adds a capture_unexpected_calls option for
FakeSwift which defauts to True. This allows the reconciler tests to
opt out of the new stricter call capturing.
Change-Id: Idc6b6b5a2b665538e861700f5d0996fc39368f5b

Apparently errno.EUCLEAN is not available on OS X, which can complicate
running unit tests.
Change-Id: Iaa3d7756949b4a67d4afe8a53b242ed9f41e9374

When Expirer is iterating task containers and doing listings, it's
possible one of the container nodes hosting the task container may
become overloaded (or have a really low backend ratelimit set).
Object-expirer should expect UnexpectedResponse and continue to try
and list the task objects in the next container.
And if the task container doesn't exist, expirer should not try to
delete the non-existent containers, before continue to work on
the next container.
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Change-Id: Id1966fa22725a02471e2d7c5a42fb243b1cfcf6a

Previously the object-expirer would not respect an internal_client_conf_path option if its config was loaded from a "legacy" config filepath (e.g. **/object-expirer.conf). Legacy object-expirer config expected config sections for an internal-client to be included in the same file. However, "modern" object-expirer config files (e.g. **/object-server.conf) expect internal-client config to be loaded from a separate file specified by internal_client_conf_path and defaulting to 'internal_client_conf_path'.
"Legacy" expirer config is still used in production clusters but it is useful to use a shared internal-client config. This patch therefore makes the internal_client_conf_path option always be respected regardless of the path of the object-expirer conf file. If 'internal_client_conf_path' is not specified, "modern" config will continue to use the default '/etc/swift/internal-client.conf', but "legacy" config will default to the path of the expirer conf file (i.e. the same as the previous behavior).
Related-Change: Ib21568f9b9d8547da87a99d65ae73a550e9c3230
Change-Id: I24ec702cd2ed074ca9df084cefc896418cece394

py2 http responses don't have a 'headers' attribute, so don't try to
use it.
Unfortunately the unit test infrastructure's FakeConn class does
have a 'headers' attribute (whose value doesn't match the result of
calling its 'getheaders()' method!), so this bug was not caught by
tests in the Related-Change.
Related-Change: I96f28ab0b2b5f9374c399e8905ee240e7b093f8b
Change-Id: I2cd820280b8c69cafc5730183903c9d379d8dde5

The RelatedChange introduced a _make_internal_client() method with an
unused argument 'is_legacy_conf'. This patch completes the original
intention i.e. for the selection of internal client config path to
also be moved to the new method and use the 'is_legacy_conf' arg.
Change-Id: I5075cb446a15edc7f47e83f6aa038c626bd1dd82
RelatedChange: Ia6e1e6a8b58a8476fa16a3c7d45e620c6d7f88e4

Note that you can still have a "//" in the path with rest_with_last, though.
Change-Id: I171afcd67b162634189b752ff92a4f43484bc12a