7 Commits

Author SHA1 Message Date
Samuel Merritt
0e81ffd1e1 Fix socket leak on object-server death
Consider a client that's downloading a large replicated object of size
N bytes. If the object server process dies (e.g. with a segfault)
partway through the download, the proxy will have read fewer than N
bytes, and then read(sockfd) will start returning 0 bytes. At this
point, the proxy believes the object download is complete, and so the
WSGI server waits for a new request to come in. Meanwhile, the client
is waiting for the rest of their bytes. Until the client times out,
that socket will be held open.

The fix is to look at the Content-Length and Content-Range headers in
the response from the object server, then retry with another object
server in case the original GET is truncated. This way, the client
gets all the bytes they should.

Note that ResumingGetter already had retry logic for when an
object-server is slow to send bytes -- this extends it to also cover
unexpected disconnects.

Change-Id: Iab1e07706193ddc86832fd2cff0d7c2cb6d79ad9
Related-Change: I74d8c13eba2a4917b5a116875b51a781b33a7abf
Closes-Bug: 1568650
2019-01-31 18:38:35 +00:00
Alistair Coles
511ac2ee60 Use setUpModule instead of setup for module level unit test setup
Module setup() and teardown() functions are found by nosetests [1] but
unittests expects setUpModule() and tearDownModule() [2]. The latter
function names are also found by nosetests, so using those function
names enables the test module to be run with either nosetests or
unittest.

Although the tox test envs and .unittests script use nosetests, this
change allows the convenience of using unittest, for example when it
is the default test runner in a development environment such as
PyCharm.

This change also makes it unnecessary to explicitly call the setup()
and teardown() functions when executing the module directly.

[1] http://nose.readthedocs.io/en/latest/writing_tests.html#test-modules
[2] https://docs.python.org/2/library/unittest.html#setupmodule-and-teardownmodule

Change-Id: Ib2e5470a339af1f937b25d643b64356e8848ed36
2017-05-04 12:47:17 +01:00
Kota Tsuyuzaki
8fe4bfefaa TestObjectController refactoring
From the related change of ECDuplication, Swift have a couple of Test
classes for EC policy, normal EC and EC Duplication, in the
test/unit/proxy/test_server.py. To enable the classes, the related change
abstracts the EC test cases as the ECTestMixin class to gather test
methods into one place but it was worse because TestObjectController did
still have both test cases for replication and for ec that may be hard
to understand the test class structure.

Hence, this patch attempts to refactor the structure as

From:

     ECTestMixin
            |
    -------------------------------------
    |                                   |
TestObjectController           TestObjectControllerECDuplication
(for replication and EC)       (for EC Duplication Policy)

To:

    BaseTestObjectController
            |
    --------------------------------------
    |                                    |
TestReplicatedObjectController  BaseTestECObjectController
(for replication)                        |
                          ---------------------------------
                          |                               |
                TestECObjectController    TestECDuplicationObjectController
                (for EC policy)           (for EC Duplication Policy)

Some more cleanups are in follow up patches because this patch shows a lot
of moving code chunks which could be hard to compare the diff. To make
the review easy, this patch forcus on ONLY the structure changes as
possible.

Related-Change: Idd155401982a2c48110c30b480966a863f6bd305
Related-Change: I25a3f8fc837706d78dca226fe282d9e5ead65a0d
Change-Id: Ifd3d0fa66773e640bb61cc528f7a1b2358e97d91
2017-03-22 19:54:50 +00:00
Kota Tsuyuzaki
1eb96397e7 Fix EC ring validation at ring reload
Swift EC has a strong constraint about the ring must have a number of
replicas which fits ec_k + ec_m. That is validated when servers waking
up. However, Swift has more chance to load such an invalid ring when
a request comming, calling some node iteration like get_nodes,
get_part_nodes or so, and no ring validation is there.

This patch moves ring validation from policy validate_ring into the ring
instance as validation_hook that will run at ring reload. Since this patch,
ring instance will allow to use the old ring if the reload is not fourced.

Note that the exception if invalid ring found was changed from
RingValidationError to RingLoadError because RingValidationError is a
child of RingBuilderError but the ring reload is obviously outside of
"builder".

Closes-Bug: #1534572

Change-Id: I6428fbfb04e0c79679b917d5e57bd2a34f2a0875
2016-07-24 21:49:57 -07:00
Alistair Coles
fa89064933 Per-policy DiskFile classes
Adds specific disk file classes for EC policy types.

The new ECDiskFile and ECDiskFileWriter classes are used by the
ECDiskFileManager.

ECDiskFileManager is registered with the DiskFileRouter for use with
EC_POLICY type policies.

Refactors diskfile tests into BaseDiskFileMixin and BaseDiskFileManagerMixin
classes which are then extended in subclasses for the legacy
replication-type DiskFile* and ECDiskFile* classes.

Refactor to prefer use of a policy instance reference over a policy_index
int to refer to a policy.

Add additional verification to DiskFileManager.get_dev_path to validate the
device root with common.constraints.check_dir, even when mount_check is
disabled for use in on a virtual swift-all-in-one.

Co-Authored-By: Thiago da Silva <thiago@redhat.com>
Co-Authored-By: John Dickinson <me@not.mn>
Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
Co-Authored-By: Tushar Gohad <tushar.gohad@intel.com>
Co-Authored-By: Paul Luse <paul.e.luse@intel.com>
Co-Authored-By: Samuel Merritt <sam@swiftstack.com>
Co-Authored-By: Christian Schwede <christian.schwede@enovance.com>
Co-Authored-By: Yuan Zhou <yuan.zhou@intel.com>
Change-Id: I22f915160dc67a9e18f4738c1ddf068344e8ad5d
2015-04-14 00:52:16 -07:00
Samuel Merritt
6acea29fa6 Move all DLO functionality to middleware
This is for the same reason that SLO got pulled into middleware, which
includes stuff like automatic retry of GETs on broken connection and
the multi-ring storage policy stuff.

The proxy will automatically insert the dlo middleware at an
appropriate place in the pipeline the same way it does with the
gatekeeper middleware. Clusters will still support DLOs after upgrade
even with an old config file that doesn't mention dlo at all.

Includes support for reading config values from the proxy server's
config section so that upgraded clusters continue to work as before.

Bonus fix: resolve 'after' vs. 'after_fn' in proxy's required filters
list. Having two was confusing, so I kept the more-general one.

DocImpact

blueprint multi-ring-large-objects

Change-Id: Ib3b3830c246816dd549fc74be98b4bc651e7bace
2014-02-03 18:29:48 -08:00
Peter Portante
5202b0e586 DiskFile API, with reference implementation
Refactor on-disk knowledge out of the object server by pushing the
async update pickle creation to the new DiskFileManager class (name is
not the best, so suggestions welcome), along with the REPLICATOR
method logic. We also move the mount checking and thread pool storage
to the new ondisk.Devices object, which then also becomes the new home
of the audit_location_generator method.

For the object server, a new setup() method is now called at the end
of the controller's construction, and the _diskfile() method has been
renamed to get_diskfile(), to allow implementation specific behavior.

We then hide the need for the REST API layer to know how and where
quarantining needs to be performed. There are now two places it is
checked internally, on open() where we verify the content-length,
name, and x-timestamp metadata, and in the reader on close where the
etag metadata is checked if the entire file was read.

We add a reader class to allow implementations to isolate the WSGI
handling code for that specific environment (it is used no-where else
in the REST APIs). This simplifies the caller's code to just use a
"with" statement once open to avoid multiple points where close needs
to be called.

For a full historical comparison, including the usage patterns see:
https://gist.github.com/portante/5488238

(as of master, 2b639f5, Merge
 "Fix 500 from account-quota     This Commit
 middleware")
--------------------------------+------------------------------------
                                 DiskFileManager(conf)

                                   Methods:
                                     .pickle_async_update()
                                     .get_diskfile()
                                     .get_hashes()

                                   Attributes:
                                     .devices
                                     .logger
                                     .disk_chunk_size
                                     .keep_cache_size
                                     .bytes_per_sync

DiskFile(a,c,o,keep_data_fp=)    DiskFile(a,c,o)

  Methods:                         Methods:
   *.__iter__()
    .close(verify_file=)
    .is_deleted()
    .is_expired()
    .quarantine()
    .get_data_file_size()
                                     .open()
                                     .read_metadata()
    .create()                        .create()
                                     .write_metadata()
    .delete()                        .delete()

  Attributes:                      Attributes:
    .quarantined_dir
    .keep_cache
    .metadata
                                *DiskFileReader()

                                   Methods:
                                     .__iter__()
                                     .close()

                                   Attributes:
                                    +.was_quarantined

DiskWriter()                     DiskFileWriter()

  Methods:                         Methods:
    .write()                         .write()
    .put()                           .put()

* Note that the DiskFile class   * Note that the DiskReader() object
  implements all the methods       returned by the
  necessary for a WSGI app         DiskFileOpened.reader() method
  iterator                         implements all the methods
                                   necessary for a WSGI app iterator

                                 + Note that if the auditor is
                                   refactored to not use the DiskFile
                                   class, see
                                   https://review.openstack.org/44787
                                   then we don't need the
                                   was_quarantined attribute

A reference "in-memory" object server implementation of a backend
DiskFile class in swift/obj/mem_server.py and
swift/obj/mem_diskfile.py.

One can also reference
https://github.com/portante/gluster-swift/commits/diskfile for the
proposed integration with the gluster-swift code based on these
changes.

Change-Id: I44e153fdb405a5743e9c05349008f94136764916
Signed-off-by: Peter Portante <peter.portante@redhat.com>
2013-10-17 15:03:31 -04:00