Sage Weil [Tue, 16 Jul 2013 22:28:07 +0000 (15:28 -0700)]
osd/OSDMonitor: make 'osd pool rmsnap ...' not racy/crashy
Ensure that the snap does in fact exist before we try to remove it. This
avoids a crash where a we get two dup rmsnap requests (due to thrashing, or
a reconnect, or something), the committed (p) value does have the snap, but
the uncommitted (pp) does not. This fails the old test such that we try
to remove it from pp again, and assert.
Restructure the flow so that it is easier to distinguish the committed
short return from the uncommitted return (which must still wait for the
commit).
0> 2013-07-16 14:21:27.189060 7fdf301e9700 -1 osd/osd_types.cc: In function 'void pg_pool_t::remove_snap(snapid_t)' thread 7fdf301e9700 time 2013-07-16 14:21:27.187095
osd/osd_types.cc: 662: FAILED assert(snaps.count(s))
ceph version 0.66-602-gcd39d8a (cd39d8a6727d81b889869e98f5869e4227b50720)
1: (pg_pool_t::remove_snap(snapid_t)+0x6d) [0x7ad6dd]
2: (OSDMonitor::prepare_command(MMonCommand*)+0x6407) [0x5c1517]
3: (OSDMonitor::prepare_update(PaxosServiceMessage*)+0x1fb) [0x5c41ab]
4: (PaxosService::dispatch(PaxosServiceMessage*)+0x937) [0x598c87]
5: (Monitor::handle_command(MMonCommand*)+0xe56) [0x56ec36]
6: (Monitor::_ms_dispatch(Message*)+0xd1d) [0x5719ad]
7: (Monitor::handle_forward(MForward*)+0x821) [0x572831]
8: (Monitor::_ms_dispatch(Message*)+0xe44) [0x571ad4]
9: (Monitor::ms_dispatch(Message*)+0x32) [0x588c52]
10: (DispatchQueue::entry()+0x549) [0x7cf1d9]
11: (DispatchQueue::DispatchThread::entry()+0xd) [0x7060fd]
12: (()+0x7e9a) [0x7fdf35165e9a]
13: (clone()+0x6d) [0x7fdf334fcccd]
NOTE: a copy of the executable, or `objdump -rdS <executable>` is needed to interpret this.
Signed-off-by: Sage Weil <sage@inktank.com> Reviewed-by: Joao Eduardo Luis <joao.luis@inktank.com>
mon: OSDMonitor: only thrash and propose if we are the leader
'thrash_map' is only set if we are the leader, so we would thrash and
propose the pending value if we are the leader. However, we should keep
the 'is_leader()' check not only for clarity's sake (an unfamiliar reader
may cry OMGBUG, prompting to a patch much like this), but also because
we may lose a subsequent election and become a peon instead, while still
holding a 'thrash_map' value > 0 -- and we really don't want to propose
while being a peon.
Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com> Reviewed-by: Sage Weil <sage@inktank.com>
Sage Weil [Tue, 16 Jul 2013 21:49:33 +0000 (14:49 -0700)]
mon/OSDMonitor: send_to_waiting() in on_active()
The send_latest() helper may put a message in the waiting_for_map list
if we are not readable, but currently send_to_waiting() is only called
from update_from_paxos(), and it is possible that we may be unreadable
but not get a map update.
Instead, share the map when we are active. Do the same for check_subs(),
which is also about sharing the *new* map. Leave
share_map_with_random_osd() and process_failures() which are not
concerned with whether this is the latest map or not.
Fixes: #5439
ECANCELED there means that we lost in a race to write the object. We
should treat it as a successful write. This is reviving an old behavior
that was changed inadvertently.
Sage Weil [Tue, 16 Jul 2013 20:14:50 +0000 (13:14 -0700)]
ceph-disk: rely on /dev/disk/by-partuuid instead of special-casing journal symlinks
This was necessary when ceph-disk-udev didn't create the by-partuuid (and
other) symlinks for us, but now it is fragile and error-prone. (It also
appears to be broken on a certain customer RHEL VM.) See d7f7d613512fe39ec883e11d201793c75ee05db1.
Instead, just use the by-partuuid symlinks that we spent all that ugly
effort generating.
Backport: cuttlefish Signed-off-by: Sage Weil <sage@inktank.com> Reviewed-by: Dan Mick <dan.mick@inktank.com>
mon: Monitor: StoreConverter: clearer debug message on 'needs_conversion()'
The previous debug message outputted the function's name, as often our
functions do. This was however a source of bewilderment, as users would
see those in logs and think their stores would need conversion. Changing
this message is trivial enough and it will make ceph users happier log
readers.
Backport: cuttlefish Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com> Reviewed-by: Sage Weil <sage@inktank.com>
mon: Monitor: do not reopen MonitorDBStore during conversion
We already open the store on ceph_mon.cc, before we start the conversion.
Given we are unable to reproduce this every time a conversion is triggered,
we are led to believe that this causes a race in leveldb that will lead
to 'store.db/LOCK' being locked upon the open this patch removes.
Regardless, reopening the db here is pointless as we already did it when
we reach Monitor::StoreConverter::convert().
Fixes: #5640
Backport: cuttlefish
Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com> Reviewed-by: Sage Weil <sage@inktank.com>
When we set bucket.instance meta, we need to set
the correct bucket placement to the bucket (according to
the specific placement rule). However, it might be that
bucket placement was never configured and we just go by
the defaults, using the old legacy pools selection.
Sage Weil [Sun, 14 Jul 2013 22:54:29 +0000 (15:54 -0700)]
messages/MClientReconnect: clear data when encoding
The MClientReconnect puts everything in the data payload portion of
the message and nothing in the front portion. That means that if the
message is resent (socket failure or something), the messenger thinks it
hasn't been encoded yet (front empty) and reencodes, which means
everything gets added (again) to the data portion.
Decoding keep decoding until it runs out of data, so the second copy
means we decode garbage snap realms, leading to the crash in bug
Clearing data each time around resolves the problem, although it does
mean we do the encoding work multiple times. We could alternatively
(or also) stick some data in the front portion of the payload
(ignored), but that changes the wire protocol and I would rather not
do that.
Sage Weil [Sat, 13 Jul 2013 04:52:30 +0000 (21:52 -0700)]
mon: set forwarded message recv stamp
Set it to the stamp of the MForward that carried us. One could argue
we really want the original receive stamp on the origin, but that is
not available to us, and this is better than nothing.
In particular, this gives 'ceph log ...' commands a timestamp when they
are forwarded via a peon. The stamp is still between when the request
is sent and when it is committed/acked, so all is well from the
client's perspective.
Sage Weil [Sat, 13 Jul 2013 15:36:25 +0000 (08:36 -0700)]
mon: set peon state to electing if other mons call an election
Previously we would call mon->reset() and set various flags (like
exited_quorum timestamp), but the state would remain PEON. Make an
explicit join_election() callback and set the state there, and add
asserts in reset() (renamed to be private) so that we ensure all
callers are well-behaved.
Sage Weil [Sat, 13 Jul 2013 15:11:45 +0000 (08:11 -0700)]
mon: once sync full is chosen, make sure we don't change our mind
It is possible for a sequence like:
- probe
- first probe reply has paxos trim that indicates a full sync is
needed
- start sync
- clear store
- something happens that makes us abort and bootstrap (e.g., the
provider mon restarts
- probe
- first probe reply has older paxos trim bound and we call an election
- on election completion, we crash because we have no data.
Non-determinism of the probe decision aside, we need to ensure that
the info we share during probe (fc, lc) is accurate, and that once we
clear the store we know we *must* do a full sync.
Fixes: #5621
Backport: cuttlefish Signed-off-by: Sage Weil <sage@inktank.com>
Sage Weil [Mon, 15 Jul 2013 19:56:26 +0000 (12:56 -0700)]
mon/PaxosService: consolidate resetting in restart()
We had duplicated code in election_finished() and restart(), and it was
incomplete. Put it all in restart() only (the mon should have called
restart() long before the election finishes). Note that we cannot
assert as much in election_finished() because another service may have
just cross-proposed.
Sage Weil [Fri, 12 Jul 2013 21:47:09 +0000 (14:47 -0700)]
mon: fix scrub vs paxos race: refresh on commit, not round completion
Consider:
- paxos starts a commit N+1
- a majority of the peers ack it
- paxos::commit() writes N+1 it to disk
- tells peers to commit
- peers commit N+1, *and* refresh_from_paxos(), and generate N+1 full map
- leader does _scrub on N+1, without latest full osdmap
- peers do _scrub on N+1, with latest full osdmap
- leader finishes paxos gather, does refresh_from_paxos()
-> scrub fails.
Fix this by doing the refresh_from_paxos() at commit time and not when
the paxos round finishes. We move the refresh out of finish_proposal
and into its own helper, and update all callers accordingly. This
keeps on-disk state more tightly in sync with in-memory state and
avoids the need for a e.g., kludgey workaround in the scrub code.
We also simplify the bootstrap checks a bit by doing so immediately
and relying on the normal bootstrap paxos reset paths to clean up
any waiters.
Sage Weil [Sat, 13 Jul 2013 15:11:45 +0000 (08:11 -0700)]
mon: once sync full is chosen, make sure we don't change our mind
It is possible for a sequence like:
- probe
- first probe reply has paxos trim that indicates a full sync is
needed
- start sync
- clear store
- something happens that makes us abort and bootstrap (e.g., the
provider mon restarts
- probe
- first probe reply has older paxos trim bound and we call an election
- on election completion, we crash because we have no data.
Non-determinism of the probe decision aside, we need to ensure that
the info we share during probe (fc, lc) is accurate, and that once we
clear the store we know we *must* do a full sync.
Sage Weil [Mon, 15 Jul 2013 16:58:08 +0000 (09:58 -0700)]
rgw: fix more warnings
test/test_rgw_admin_opstate.cc: In member function 'int admin_log::test_helper::extract_input(int, char**)':
warning: test/test_rgw_admin_opstate.cc:129:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_opstate.cc:131:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_opstate.cc:133:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_opstate.cc:135:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
test/test_rgw_admin_log.cc: In member function 'int admin_log::test_helper::extract_input(int, char**)':
warning: test/test_rgw_admin_log.cc:132:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_log.cc:134:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_log.cc:136:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_log.cc:138:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
Sage Weil [Sun, 14 Jul 2013 23:37:45 +0000 (16:37 -0700)]
test_rgw_admin_meta: fix warnings
test/test_rgw_admin_meta.cc: In member function 'int admin_meta::test_helper::extract_input(int, char**)':
warning: test/test_rgw_admin_meta.cc:126:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_meta.cc:128:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_meta.cc:130:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
warning: test/test_rgw_admin_meta.cc:132:24: comparison between signed and unsigned integer expressions [-Wsign-compare]
Sage Weil [Sun, 14 Jul 2013 23:36:21 +0000 (16:36 -0700)]
cls_rgw: fix warning
cls/rgw/cls_rgw.cc: In function 'int get_obj_vals(cls_method_context_t, const string&, const string&, int, std::map, ceph::buffer::list>*)':
warning: cls/rgw/cls_rgw.cc:175:28: narrowing conversion of '129' from 'int' to 'char' inside { } is ill-formed in C++11 [-Wnarrowing]
Sage Weil [Fri, 12 Jul 2013 20:50:49 +0000 (13:50 -0700)]
mon/PaxosService: do not prepare new pending if still proposing
The _active callback can get called while are already proposing. If
that happens, we should not prepare a fresh new pending but should
wait for the previous proposal to finish.
Signed-off-by: Sage Weil <sage@inktank.com> Reviewed-by: Greg Farnum <greg@inktank.com>
Sage Weil [Sat, 13 Jul 2013 04:42:19 +0000 (21:42 -0700)]
mon/PaxosService: fix trim completion
Do not call C_Committed after trim or else we will prematurely clear
the bool proposing, propose something again using the same version, and
crash. We do not in fact need anything to happen here aside from the
refresh_from_paxos() that happens on its own.
Dan Mick [Sat, 13 Jul 2013 04:23:14 +0000 (21:23 -0700)]
ceph_argparse.py: allow valid char RE arg to CephString
Change badchars to goodchars (no one was using badchars); allow
goodchars to be a RE character class of valid characters for the
param. First use: crush item names.
Dan Mick [Fri, 12 Jul 2013 20:58:36 +0000 (13:58 -0700)]
ceph-rest-api: separate into module and front-end for WSGI deploy
To deploy ceph-rest-api within a WSGI server (apache/mod_wsgi,
nginx/uwsgi, etc.), there needs to be an importable (.py) module
that performs all init/config when imported. ceph-rest-api was
close, but it needs to be named properly, and there's no argument
passing, so it needs to get args from a fixed file or the env.
Separate most of ceph-rest-api into pybind/ceph_rest_api.py, and make
its arguments come from the environment, and init errors be
ImportError exceptions. Recase ceph-rest-api as a thin layer that
does the usual setup and arg parsing, and then sets args into the
environment and imports ceph_rest_api.py, catching exceptions and
reporting errors. This allows standalone execution as usual.
ceph-rest-api grabs a few module globals (addr/port and the flask.app)
to use after it imports.
Accept cluster name, and do the ceph.conf search using cluster name
in the appropriate places in the searched-for files.
Also ceph_rest_api.py gets a little cleanup (fewer global variables,
cleaner conf file search algorithm, better error reporting on conf
load)
Also: doc updates, packaging updates to include ceph_rest_api.py
Sage Weil [Fri, 12 Jul 2013 23:21:24 +0000 (16:21 -0700)]
msg/Pipe: fix RECONNECT_SEQ behavior
Calling handle_ack() here has no effect because we have already
spliced sent messages back into our out queue. Instead, pull them out
of there and discard. Add a few assertions along the way.
Signed-off-by: Sage Weil <sage@inktank.com> Reviewed-by: Greg Farnum <greg@inktank.com>
Sage Weil [Fri, 12 Jul 2013 20:12:51 +0000 (13:12 -0700)]
mon: AuthMonitor: don't try to auth import a null buffer
Hangs result if 'ceph auth import' is attempted without -i.
Check for this case and return error status. Also,
update auth import help to more-clearly indicate that "input"
means "-i <file>".
Fixes: #4599 Signed-off-by: Dan Mick <dan.mick@inktank.com> Reviewed-by: Sage Weil <sage@inktank.com>
David Zafman [Fri, 12 Jul 2013 02:47:47 +0000 (19:47 -0700)]
test: idempotent filestore test failure
Remove obsolete use of collection_move()
Allow operations to be skipped if random selections don't make sense
Track total number of possible objects in m_num_objects
BUG: do_remove() was calling _do_touch() instead of _do_remove()
For ops that require an object, select from among existing objects in collection
Initialize m_num_objects unique objects across collections
touch: don't create an object that already exists in another collection
remove: Use remove_obj() to clear object from m_objects to have accurate tracking
clone/clone_range(): Select 2 existing objects in the collection
add: Skip operation if selected target object name exists in target collection
move: Removed this buggy operation that is only present for upgrades
Fixes: #5371 Fixes: #5240 Signed-off-by: David Zafman <david.zafman@inktank.com>