New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osd: simplify past_intervals representation #14444

Merged
merged 43 commits into from May 2, 2017

Conversation

Projects
None yet
4 participants
@liewegas
Member

liewegas commented Apr 10, 2017

This is based on Sam's branch, but with many many changes and fixes. It's now passing the rados suite. Ready for another review!

@liewegas liewegas changed the title from osd: simplify past_intervals representation to DNM osd: simplify past_intervals representation Apr 14, 2017

@liewegas liewegas changed the title from DNM osd: simplify past_intervals representation to osd: simplify past_intervals representation Apr 20, 2017

@liewegas liewegas requested a review from jdurgin Apr 20, 2017

@jdurgin

that is a lot of bugs fixed. lgtm modulo signed-off-by-check failure for the 'fix messages/:' commit

@jdurgin jdurgin added the needs-qa label Apr 28, 2017

athanatos and others added some commits Dec 9, 2016

PG::GetInfo: remove logic for detecting intervals without complete peers
c7d92d1 introduced this back when
the acting set could contain incomplete peers during backfill.  That
hasn't been true since dumpling.  Now, any interval where the acting
set contains an incomplete peer cannot possibly go active.  Thus, it
can't change last_epoch_started or history.last_epoch_started.  Thus,
even though choose_acting omits incomplete peers, the answer can't
change.

Signed-off-by: Samuel Just <sjust@redhat.com>
osd/: refactor past_intervals logic into PastIntervals
Signed-off-by: Samuel Just <sjust@redhat.com>
osd/: hide pg_interval_t encoder
Signed-off-by: Samuel Just <sjust@redhat.com>
messages/: accomodate the new past_intervals encoding
Signed-off-by: Samuel Just <sjust@redhat.com>
RadosDump: accommodate new past_intervals encoding
Signed-off-by: Samuel Just <sjust@redhat.com>
PG: accommodate new past_intervals encoding
Signed-off-by: Samuel Just <sjust@redhat.com>
osd_types: implement PastIntervals pi_compact_rep
Signed-off-by: Samuel Just <sjust@redhat.com>
osd_types: fill in generate_past_intervals for new types
Signed-off-by: Samuel Just <sjust@redhat.com>
osd_types: just pass last_epoch_started to PriorSet
Signed-off-by: Samuel Just <sjust@redhat.com>
test/osd_types.cc: fix emacs line at the top
Signed-off-by: Samuel Just <sjust@redhat.com>
qa/suites/upgrade/jewel-x: add cache tiering + snaps workload
Signed-off-by: Sage Weil <sage@redhat.com>
osd_types: refactor PriorSet so it can be tested without an OSDMap
Signed-off-by: Samuel Just <sjust@redhat.com>
test/osd/types: add tests specifically for prior set generation
Signed-off-by: Samuel Just <sjust@redhat.com>
test/osd/types.cc: fix check_new_interval test
Removes the asserts relating to past_intervals state.
They don't really make sense with the new representation,
we'll add new tests for that next.

Signed-off-by: Samuel Just <sjust@redhat.com>
unittest_osd_types: minor fix to past_intervals tests
Set [up_]primary to valid values (not that it is actually
relevant to the test!).

Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: log peering intervals at high debug level
Because we are no longer explicitly tracking all past intervals
I think it is a good idea to log these more aggressively.  This will
give us potentially vital information when debugging peering problems
(says someone who just debugged a peering problem in which the past
interval information provided a vital clue).

Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: put interval bounds [) in debug/log output
Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: fix pi_compact_rep bounds; rename fields
The second bound is non-inclusive.  Use 'first' and 'last' instead of
'start' and 'end' to reinforce that.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: fix pi_compact_rep empty() condition
We may have no intervals but still be non-empty (have a first and
last), because during that period there were no osds.  This easily
happens when a pool is created before osds are up.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: simplify bounds
The bounds are based on last_epoch_clean, which can
happen at any point during an interval (usually not the
beginning!).  Instead of trying to ensure that the
PastIntervals include the oldest interval, just ensure
that they go at least as far back as last_epoch_clean.
This means that we might have *more* intervals, but given
that all we ever do is *clear* past_intervals when we
go clean, I don't think there is much value in trying
assert more.

Signed-off-by: Sage Weil <sage@redhat.com>

liewegas added some commits Apr 11, 2017

messages/MOSDPGNotify: streamline luminous+ encoding
Signed-off-by: Sage Weil <sage@redhat.com>
messages/MOSDPGLog: show past_intervals if present
Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: print and encode empty PastIntervals
These happen when they are optionally sent across the
wire.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: fix PastIntervals::update_type()
Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: fix encoding for PastIntervals
Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: more compact compact_interval_t printer
Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: allow get_bounds() on empty
Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: track pg_history last_interval_{starte,clean}
These will track last_epoch_{started,clean} but match
the first epoch in the interval instead of the epoch when
the event happened.  We didn't end up need this now, but
I suspect it will be useful in the future.

Signed-off-by: Sage Weil <sage@redhat.com>
osd: populate PastIntervals on new PGs
When we get a pg create from the mon we don't get
PastIntervals with it; generate it from scratch as
needed.

Signed-off-by: Sage Weil <sage@redhat.com>
@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 28, 2017

@liewegas needs rebase

liewegas added some commits Apr 11, 2017

osd/PG: store last_interval_started in pg_info_t too
We have a last_epoch_started value in pg_info_t; store
the corresponding last_interval_started value alongside
it.

Signed-off-by: Sage Weil <sage@redhat.com>
src/test/vstart-wrapper.sh: run with -d
Signed-off-by: Sage Weil <sage@redhat.com>
osd: fix build_past_intervals_parallel
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: consolidate all history updates in update_history
...and take care of reg and unreg_scrub.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: clear past_intervals from update_history
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: dirty_big_info=true when we clear past_intervals
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: clear past_intervals as needed on upgrade
Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: clear past intervals on a osdmap gap
If we are crossing an osdmap gap, clear our old past
interval since the information is stale: someone else
took the pg clean after everything we know if the maps
were trimmed.

This avoids an assert later in the past intervals check.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/osd_types: encode all_participants for pi_compact
The all_participants is only updated on add_interval and
cannot be rebuilt from the intervals vector alone since
we only keep the smallset subset of peers to probe that
we need.  And it wasn't being encoded/decoded.  Or
dumped.

Signed-off-by: Sage Weil <sage@redhat.com>
osd/PG: fix upgrade()
We need to update the infover_key on any upgrade.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas

This comment has been minimized.

Member

liewegas commented Apr 30, 2017

/a/sage-2017-04-29_22:06:05-rados-wip-sage-testing2---basic-smithi/1082864

2017-04-30T00:45:42.066 INFO:tasks.ceph.osd.5.smithi062.stderr:     0> 2017-04-30 00:45:42.031835 7f478f867700 -1 /build/ceph-12.0.1-1773-g230a4d7/src/osd/osd_types.cc: In function 'static bool PastIntervals::check_new_interval(int, int, const std::vector&, const std::vector&, int, int, const std::vector&, const std::vector&, epoch_t, epoch_t, OSDMapRef, OSDMapRef, pg_t, IsPGRecoverablePredicate*, PastIntervals*, std::ostream*)' thread 7f478f867700 time 2017-04-30 00:45:42.029512
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr:/build/ceph-12.0.1-1773-g230a4d7/src/osd/osd_types.cc: 3503: FAILED assert(lastmap->get_pools().count(pgid.pool()))
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr:
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: ceph version 12.0.1-1773-g230a4d7 (230a4d79ff6ef8a62bdb8f258f92c58bc7496fee)
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: 1: (ceph::__ceph_assert_fail(char const*, char const*, int, char const*)+0x10e) [0x555aadee085e]
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: 2: (PastIntervals::check_new_interval(int, int, std::vector > const&, std::vector > const&, int, int, std::vector > const&, std::vector > const&, unsigned int, unsigned int, std::shared_ptr, std::shared_ptr, pg_t, IsPGRecoverablePredicate*, PastIntervals*, std::ostream*)+0x58f) [0x555aadbd7f6f]
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: 3: (OSD::build_initial_pg_history(spg_t, unsigned int, utime_t, pg_history_t*, PastIntervals*)+0x594) [0x555aad9dd674]
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: 4: (OSD::handle_pg_create(boost::intrusive_ptr)+0xb24) [0x555aad9e7ff4]
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: 5: (OSD::dispatch_op(boost::intrusive_ptr)+0x1b1) [0x555aad9e9f91]
2017-04-30T00:45:42.067 INFO:tasks.ceph.osd.5.smithi062.stderr: 6: (OSD::_dispatch(Message*)+0x377) [0x555aad9ea927]
2017-04-30T00:45:42.068 INFO:tasks.ceph.osd.5.smithi062.stderr: 7: (OSD::ms_dispatch(Message*)+0x74) [0x555aad9ead54]
2017-04-30T00:45:42.068 INFO:tasks.ceph.osd.5.smithi062.stderr: 8: (DispatchQueue::entry()+0x78b) [0x555aae0ac5bb]
2017-04-30T00:45:42.068 INFO:tasks.ceph.osd.5.smithi062.stderr: 9: (DispatchQueue::DispatchThread::entry()+0xd) [0x555aadf58f1d]

@liewegas

This comment has been minimized.

Member

liewegas commented May 1, 2017

previous failure was due to http://tracker.ceph.com/issues/19814; waiting for that fix to mege.

@liewegas liewegas merged commit fcd64d7 into ceph:master May 2, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@liewegas liewegas deleted the liewegas:wip-past-intervals branch May 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment