As described in bug #1558754 a rebalance after removing a device fails
if min_part_seconds_left > 0, despite the note that a rebalance should
remove partitions from removed devices on the next run.
This patch skips the early exit, and lets the builder itself handling
the rebalance. Partitions that shouldn't move now are still not moved
(except for removed devices), and there is still a warning if no
partition has been moved due to the fact that min_part_hours did not yet
pass.
A small test has been added to ensure rebalancing after removing a
device works without using the --force option (tests fails on current
master). Another test ensures that a rebalance after a recent change
(for example increasing a device's weight) does not move partitions and
still reports the former warning message.
Closes-Bug: 1558754
Change-Id: I083022d066338cbe6234bab491c7a8e8e0a7b517
Currently swift ignores a lot of the Hacking style guide. This patch
enables the H401 and H403 checks and fixes any violations. With this
we can get a little closer to following the OpenStack style guidelines.
Change-Id: I5109a052f2ceb2e6a9a174cded62f4231156d39b
I changed asserts with more specific assert methods.
e.g.: from assertTrue(sth == None) to assertIsNone(*) or
assertTrue(isinstance(inst, type)) to assertIsInstace(inst, type)
or assertTrue(not sth) to assertFalse(sth).
The code gets more readable, and a better description will be shown on fail.
Change-Id: I39305808ad2349dc11a42261b41dbb347ac0618a
Adjusted width of ip and port columns in swift-ring-builder command
output to dynamically span to the longest ip or the longest port in
the devices list. Also combined the port and ip address columns for
better visual clarity. Took care of ipv6 format [ipv6]:port
Modified the corresponding test case with expected output.
Change-Id: I65837f8fed70be60b53d5a817a4ce529ad0f070e
Closes-Bug: #1567105
pretend_min_part_hours_passed do things like this:
self._last_part_moves[part] = 0xff
this will throw exception if self._last_part_moves is None.
this patch is to check self._last_part_moves to prevent exception.
Closes-bug: #1578835
Change-Id: Ic83c7a338b45bfcf61f5ab6100e6db335c3fa81a
In test_ringbuilder.py, there is one assertTrue should be
replaced with assertEqual.
Change-Id: I9a0e4a7363a5e16cc9b6df045953dfbb4f9dbd07
Closes-bug: #1604320
Also added a Timeout class to test.unit to wrap possible long-running
functions. For example, if there is some regression and the "--yes"
argument is no longer evaluated correctly and the test excepts some
keyboard input, it will be terminated after a few seconds to ensure
there is no long-running blocker on the gate.
Change-Id: I07b17d21d5af7fcc594ce5319ae2b6f7f58df4bb
If one has an object.builder file in the current directory and runs
test_ringbuilder, it will fail with an irritating error. That's because
test_use_ringfile_as_builderfile doesn't use self.tmpfile, but
object.builder - and that one might exist in the local directory.
This patch changes this, using self.tmpfile as argument name.
Closes-Bug: 1590356
Change-Id: I4b3287a36e8a5e469eb037128427dc7867910e53
Changing the recommended ports for Swift services
from ports 6000-6002 to unused ports 6200-6202;
so they do not conflict with X-Windows or other services.
Updated SAIO docs.
DocImpact
Closes-Bug: #1521339
Change-Id: Ie1c778b159792c8e259e2a54cb86051686ac9d18
Some of tests in test/unit/cli/test_ringbuilder doesn't assert
the exit code and unfortunately some of these passed even if the
statement fails for the assertion actually.
This patch enables to assert the exit code from ringbuider and
fixes some code/test bugs I noticed.
Change-Id: I18fa675ba8a90678e2b5ccb5f90eafab01d22787
This patch makes test_ringbuilder create a temporal directory,
run ring builder commands under it, and delete it after testing
for each test cases, to fix temp file leaking.
Change-Id: I6f59fe095ea6485af0e60b5a8e8fc3892e0a0f90
Try to find ring file, load and compare it with builder file, then show result state.
Examples:
Ring file object.ring.gz not found, probably it hasn't been written yet
Ring file object.ring.gz is up-to-date
Ring file object.ring.gz is obsolete
Ring file object.ring.gz is invalid: ValueError('string length not a multiple of item size',)
Change-Id: I4d769aa5fe1c2b1167ec088aa372874f7d13ae48
swift-ring-builder currently only displays min_part_hours and
not the amount of time remaining before a rebalance can occur.
This information is readily available and has been displayed
as a quality of life improvement.
Additionally, a bug where the time since the last rebalance
was always updated when rebalance was called regardless of
if any partitions were reassigned. This can lead to partitions
being unable to be reassigned as they never age according to
the time since last rebalance.
Change-Id: Ie0e2b5e25140cbac7465f31a26a4998beb3892e9
Closes-Bug: #1526017
Added new unit tests:
test_add_device_old_missing_region
test_create_ring_number_of_arguments
test_add_duplicate_devices
test_rebalance_with_seed
test_set_overload_number_of_arguments
test_main_no_arguments
test_main_single_argument
test_main_with_safe
Modified existing unit tests to create sample ring at start of test.
This change was needed to have unit tests run correctly and demonstrate
code coverage.
test_unknown
test_search_device_number_of_arguments
test_list_parts_number_of_arguments
test_set_weight_number_of_arguments
test_set_info_number_of_arguments
test_remove_device_number_of_arguments
test_set_min_part_hours_number_of_arguments
test_set_replicas_number_of_arguments
test_set_replicas_invalid_value
Updates to handled nested mocks.
Updates to handle no exception case when SystemExit is expected.
PEP8 corrections.
Moved new tests from try blocks to use of assertRaises or call to
run_srb using exp_results with specified exit codes.
Updated run_srb to accept a dictionary of expected results. Specifically,
look for 'valid_exit_codes' to test, default to (0,1).
Change-Id: I4cf3f5f055a9babba140c68a9c7ff90b9c50ea62
test_write_builder_after_device_removal() wasn't setting a
default min_part_hours so a warnign was printed. Explicitly
adding a min_part_hours suppresses the warning
Change-Id: I6f234b72c34e066abb91f28e6eacf50e29be8842
If a device with 0 weight is tried to remove, the following rebalance
does not write changes into builder file.
Scenario:
$ swift-ring-builder object.builder set_weight --id 1 0.00
$ swift-ring-builder object.builder rebalance
Wait for moving files out of the device id=1.
$ swift-ring-builder object.builder remove --id 1
$ swift-ring-builder object.builder rebalance
In fact, the device id=1 is not removed after rebalance (must be --force used).
Change-Id: Iad5a444023eae9882a3addd7f119ff4d18559ddd
The builtin basestring type was removed in Python 3. Replace it with
six.string_types which works on Python 2 and Python 3.
Change-Id: Ib92a729682322cc65b41050ae169167be2899e2c
We should never assign multiple replicas of the same partition to the
same device - our on-disk layout can only support a single replica of a
given part on a single device. We should not do this, so we validate
against it and raise a loud warning if this terrible state is ever
observed after a rebalance.
Unfortunately currently there's a couple not necessarily uncommon
scenarios which will trigger this observed state today:
1. If we have less devices than replicas
2. If a server or zones aggregate device weight make it the most
appropriate candidate for multiple replicas and you're a bit unlucky
Fixing #1 would be easy, we should just not allow that state anymore.
Really we never did - if you have a 3 replica ring with one device - you
have one replica. Everything that iter_nodes'd would de-dupe. We
should just be insisting that you explicitly acknowledge your replica
count with set_replicas.
I have been lost in the abyss for days searching for a general solutions
to #2. I'm sure it exists, but I will not have wrestled it to
submission by RC1. In the meantime we can eliminate a great deal of the
luck required simply by refusing to place more than one replica of a
part on a device in assign_parts.
The meat of the change is a small update to the .validate method in
RingBuilder. It basically unrolls a pre-existing (part, replica) loop
so that all the replicas of the part come out in order so that we can
build up the set of dev_id's for which all the replicas of a given part
are assigned part-by-part.
If we observe any duplicates - we raise a warning.
To clean the cobwebs out of the rest of the corner cases we're going to
delay get_required_overload from kicking in until we achive dispersion,
and a small check was added when selecting a device subtier to validate
if it's already being used - picking any other device in the tier works
out much better. If no other devices are available in the tier - we
raise a warning. A more elegant or optimized solution may exist.
Many unittests did not meet the criteria #1, but the fix was straight
forward after being identified by the pigeonhole check.
However, many more tests were affected by #2 - but again the fix came to
be simply adding more devices. The fantasy that all failure domains
contain at least replica count devices is prevalent in both our ring
placement algorithm and it's tests. These tests were trying to
demonstrate some complex characteristics of our ring placement algorithm
and I believe we just got a bit too carried away trying to find the
simplest possible example to demonstrate the desirable trait. I think
a better example looks more like a real ring - with many devices in each
server and many servers in each zone - I think more devices makes the
tests better. As much as possible I've tried to maintain the original
intent of the tests - when adding devices I've either spread the weight
out amongst them or added proportional weights to the other tiers.
I added an example straw man test to validate that three devices with
different weights in three different zones won't blow up. Once we can
do that without raising warnings and assigning duplicate device part
replicas - we can add more. And more importantly change the warnings to
errors - because we would much prefer to not do that #$%^ anymore.
Co-Authored-By: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>
Related-Bug: #1452431
Change-Id: I592d5b611188670ae842fe3d030aa3b340ac36f9
Sometimes the given argument is internally altered and another filename is used
without a note to the operator. Even worse, a given .ring.gz filename is
sometimes written out as builder file, without updating the corresponding
.builder file.
There is already a method to parse the given argv and return the name of the
builder and ring file. However, it's rarely used and no warning is given to the
user if it is altered. This patch uses the already parsed builder and ring file
name instead of argv[1], and also adds a note to the user if the used filename
is differently to the one given as argument.
Closes-Bug: 1482096
Change-Id: I2f8ef23aeab8b07caaa799f7dcd57e684b4b2ad2
* replace "from cStringIO import StringIO"
with "from six.moves import cStringIO as StringIO"
* replace "from StringIO import StringIO"
with "from six import StringIO"
* replace "import cStringIO" and "cStringIO.StringIO()"
with "from six import moves" and "moves.cStringIO()"
* replace "import StringIO" and "StringIO.StringIO()"
with "import six" and "six.StringIO()"
This patch was generated by the stringio operation of the sixer tool:
https://pypi.python.org/pypi/sixer
Change-Id: Iacba77fec3045f96773d1090c0bd48613729a561
This is a tool to help developers quantify changes to the ring
builder. It takes a scenario (JSON file) describing the builder's
basic parameters (part_power, replicas, etc.) and a number of
"rounds", where each round is a set of operations to perform on the
builder. For each round, the operations are applied, and then the
builder is rebalanced until it reaches a steady state.
The idea is that a developer observes the ring builder behaving
suboptimally, writes a scenario to reproduce the behavior, modifies
the ring builder to fix it, and references the scenario with the
commit so that others can see that things have improved.
I decided to write this after writing my fourth or fifth hacky one-off
script to reproduce some bad behavior in the ring builder.
Change-Id: I114242748368f142304aab90a6d99c1337bced4c
Currently device names can be empty or start and/or end with spaces.
This can create unexpected results, for example these three commands
are all valid:
swift-ring-builder account.builder add "r1z1-127.0.0.1:6000/" 1
swift-ring-builder account.builder add "r1z1-127.0.0.1:6000/sda " 1
swift-ring-builder account.builder add "r1z1-127.0.0.1:6000/ meta" 1
This patch validates device names and prevents empty names or names
starting and/or ending with spaces.
Also fixed the test "test_warn_at_risk" - the test passed if the
exception was not raised.
Closes-Bug: 1438579
Change-Id: I811b0eae7db503279e6429d985275bbab8b29c9f
Sometimes, I get handed a builder file in a support ticket and a
question of the form "why is the balance [not] doing $thing?". When
that happens, I add a bunch of print statements to my local
swift/common/ring/builder.py, figure things out, and then delete the
print statements. This time, instead of deleting the print statements,
I turned them into debug() calls and added a "--debug" flag to the
rebalance command in hopes that someone else will find it useful.
Change-Id: I697af90984fa5b314ddf570280b4585ba0ba363c
This change modifies the swift-ring-builder and introduces new format
of sub-commands (search, list_parts, set_weight, set_info and remove)
in addition to add sub-command so that hostnames can be used in place
of an ip-address for the sub-commands.
The account reaper, container synchronizer, and replicators were also
updated so that they still have a way to identify a particular device
as being "local".
Previously this was Change-Id:
Ie471902413002872fc6755bacd36af3b9c613b74
Change-Id: Ieff583ffb932133e3820744a3f8f9f491686b08d
Co-Authored-By: Alex Pecoraro <alex.pecoraro@emc.com>
Implements: blueprint allow-hostnames-for-nodes-in-rings
...and output overload as a percent like dispersion and balance.
Also raise a warning if someone tries to set overload higher than 100%
(unless the specifically requested a percent value great than 100).
Change-Id: Id030123153ea746671a8f1ca306d4b86e903fa22
Output a dispersion report that shows how many parts have each replica count
at each tier along with some additional context. Also the max_dispersion is a
good canary for what a reasonable overload might be.
Also display a warning on rebalance if the ring's dispersion is sub-optimal.
The primitive form of the dispersion graph is cached on the builder, but the
dispersion command will build it on the fly if you have a ring that was last
rebalanced before the change.
Also add --force option to rebalance to make it write a ring even if less than
1% of parts moved.
Try to clarify some dispersion and balance a little bit in the ring section of
the architectural overview.
Co-Authored-By: Christian Schwede <christian.schwede@enovance.com>
Co-Authored-By: Darrell Bishop <darrell@swiftstack.com>
Change-Id: I7696df25d092fac56588080722e0a4167ed2c824
The ring builder's placement algorithm has two goals: first, to ensure
that each partition has its replicas as far apart as possible, and
second, to ensure that partitions are fairly distributed according to
device weight. In many cases, it succeeds in both, but sometimes those
goals conflict. When that happens, operators may want to relax the
rules a little bit in order to reach a compromise solution.
Imagine a cluster of 3 nodes (A, B, C), each with 20 identical disks,
and using 3 replicas. The ring builder will place 1 replica of each
partition on each node, as you'd expect.
Now imagine that one disk fails in node C and is removed from the
ring. The operator would probably be okay with remaining at 1 replica
per node (unless their disks are really close to full), but to
accomplish that, they have to multiply the weights of the other disks
in node C by 20/19 to make C's total weight stay the same. Otherwise,
the ring builder will move partitions around such that some partitions
have replicas only on nodes A and B.
If 14 more disks failed in node C, the operator would probably be okay
with some data not living on C, as a 4x increase in storage
requirements is likely to fill disks.
This commit introduces the notion of "overload": how much extra
partition space can be placed on each disk *over* what the weight
dictates.
For example, an overload of 0.1 means that a device can take up to 10%
more partitions than its weight would imply in order to make the
replica dispersion better.
Overload only has an effect when replica-dispersion and device weights
come into conflict.
The overload is a single floating-point value for the builder
file. Existing builders get an overload of 0.0, so there will be no
behavior change on existing rings.
In the example above, imagine the operator sets an overload of 0.112
on his rings. If node C loses a drive, each other drive can take on up
to 11.2% more data. Splitting the dead drive's partitions among the
remaining 19 results in a 5.26% increase, so everything that was on
node C stays on node C. If another disk dies, then we're up to an
11.1% increase, and so everything still stays on node C. If a third
disk dies, then we've reached the limits of the overload, so some
partitions will begin to reside solely on nodes A and B.
DocImpact
Change-Id: I3593a1defcd63b6ed8eae9c1c66b9d3428b33864
The swift-ring-builder list_parts before rebalance failed abnormally so
this patch fix the behavior. After this patch applies the behavior is
completion normally with the following messages.
Specified builder file "<builder_file>" is not rebalanced yet.
Please rebalance first.
Closes-Bug: #1399529
Change-Id: I9e5db6da85de4188915c51bc401604733f0e1b77
This patch provides the necessary error handling while unpickling
a builder file. Earlier if a builder file is empty/invalid/corrupted,
the stacktrace was shown to user with an exit code of 1. This fixes it
to show a user-friendly message and also returns the exit code of 2,
indicating there was a failure.
Change-Id: I51eb24702c422299629f8053d4591dd10f5863f8
Closes-Bug: #1370680
Add some tests for essential methods in swift-ring-builder.
Tests for removing or changing device settings are executed
with different search values to cover many possible command
line arguments.
Currently tested methods:
- create ring
- add device
- remove device
- set weight
- set info
- set min_part_hours
- set replicas
Tests use swift.common.ring.RingBuilder to verify actions.
Catching and testing output from print statements is not
tested, because this requires redirecting sys.stdout during
tests and that might have some sideeffects for testing tools.
bin/swift-ring-builder has been moved to swift/cli/ringbuilder.py
and slightly modified to work as before (mainly due to no more
existing global variables since that part of the code has been
moved inside a main() function).
Change-Id: Ia63f59a8faca1fad990784f27532ca07a2125454
