osd: fast dispatch of peering events and pg_map + osd sharded wq refactor #19973

liewegas · 2018-01-16T22:03:46Z

put everything in the fast dispatch path except for osd map processing and legacy pg
create messages
move most of the guts of ShardedOpWQ into a top-level OSDShard
kill the global pg_map and instead rely on the pg_slots in each OSDShard
several cleansup along the way
some bug fixes that seem related to pg removal but didn't surface on master (since this will merge before mimic not bothering to try to pull them out)

TODO

write doc for op and peering event ordering constraints

Review guide: This is a long patch series that represents a transition between several different approaches, eventually settling on a more siginificant rewrite of the sharded wq than I had originally planned. Sorry! That means that the intermediate solutions aren't worth reviewing carefully for correctness, but are still useful to see separately to understand the evolution of the code.

This series goes roughly like this:

fast dispatch of peering events. The first several commits, up through "osd: fast dispatch peering events" (parts 1 and 2) simplify the OSD.cc code so that peering messages translate directly into one or more events that are fed into the normal sharded work queue. This simplifies the number of paths through the code, making it easier to reason about.
Several patches change the pg creation behavior. The overall takeaway here is that there is an event property that determines whether the event that trigger a pg creation. Several events do this, including notify, log, and of course the create message from the mon. We have a new MOSDPGCreate2 message that specifies those creations in terms of spg_t instead of pg_t. Unfortunatley we have to keep around the old creation path (hadnle_pg_create) to deal with old monitors. That will eventaully go away, butwe have to have a couple releases for that.
'osd: prime pg_slots for to-be-split children' is notable for introducing a pg_slot-based way to deal with splits: instnatiate the slot for the child pgs and mark it so that we queue up events but don't process them yet. This will eventually avoids special check for "is this pg splitting?" that have to look things up, although those changes come a bit later.
Move scrub requests into teh work queue.
Kill the disk_tp! Yay! Now we only have two work queues: the big one, and one for commands (asok or tell). The latter is totally unrelated to PGs so for now let's leave it separate.
Here's where things get messy: several patches adjust (and adjust again) how the PG waiting works. This isn't worth looking at in detail; instead, look at the final result and the comments in OSD.h above _process that describe the rules around queueing events, peering vs op events, and how the requeuing behaves.
Several patches around "osd: store ec profile with final pool" resolve an issue with the idea that a PG survives until its data is deleted, even if the pool is gone. That means we have reinstantiate the PG on an OSD restart even if the pool info is missing from teh OSDmap. To that end, we stash info about deleted pools... namely the pg_num and the erasure-code-profile.
'osd: do not release recovery_ops_reserved on requeue' is notable because the old behavior made no sense to me, so i "fixed" it.. but that old behavior is still present (I think) in luminous. Please see if I'm missing something!
lots of the lookup methos in OSD.h are simplified here, and even further later on when the number of ways we look up pgs continually shrinks (yay!).
'osd: kill pg_map' is the next big shift: we remove pg_map, and rely entirely on each OSDShard's pg_slots table. This simplifies everything because we have less stuff to keep consistent in memory and one less mutex to deal with. It's a huge patch, and a lot of the code it touches gets changed later, so don't worry too much about correctness yet.
'restructure consume_map' is the next big one. The concept is that the map comes into handle_osd_map, is written to disk,a nd gets to consume_map (same as before). At this point we push it down into each shard. It's at this point that we make sure the shard's OSDMapRef and the pg_slots are in sync in terms of priming splits: once the shard's ref is updated we know that any resulting split is already primed. Later, the advance_pg() bit that pulls PGs up to the shard map can thus be sure that any splits that result then are primed and the children can be installed as soon as they're ready. The old split tracking machinery (lock and a bunch of random maps) is cleaned out, yay!
_attach_pg and _detach_pg unify the pg linkage to pg_slots in one place. Also we now have a PG pointer back to its shard and its pg_slot.
pg_slots switch to unique_ptr... they're getting big so we don't want them to get copied around on hash inserts etc. (Also, we want the pg backpointer!)
The pg_epoch tracking (to figure out the min epoch for all pgs) is moved to be per-shard. Old machinery is removed.
RecoveryCtx completions handling is simplified (attach directly to txn, yay)
The pg counts are adjusted on PG removal. This is good enough for now, I think, but I'm definitely looking for ways to improve this later.
'osd: kill broken _process optimization; simplify null pg flow' is the last biggish rewrite. The all-important _process flow is simplified with (I think) all remnants of complexity from the pg_map days removed. The checks that are needed after dropping and retaking the shard lock are also grouped and only applied when the pg is non-null; if it's null we skip it all.
The remaining patches are small fixups.

At the end, I would review the _process() code itself along with the description of ordering in OSD.h. This is the heart of it!

liewegas · 2018-01-16T23:20:03Z

@gregsfortytwo killed off the disk_tp and recovery_gen_wq at the end, too!

gregsfortytwo · 2018-01-17T01:48:57Z

src/osd/PG.cc

-void PG::update_store_on_load()
-{
-  if (osd->store->get_type() == "filestore") {
-    // legacy filestore didn't store collection bit width; fix.


This has probably already been done, but I'm not sure which feature check prevents us from booting a pre-luminous store?

_preboot() has

} else if (osdmap->require_osd_release < CEPH_RELEASE_LUMINOUS) { derr << "osdmap require_osd_release < luminous; please upgrade to luminous" << dendl;

gregsfortytwo · 2018-01-17T01:51:36Z

src/osd/PG.h

@@ -557,7 +557,7 @@ class PG : public DoutPrefixProvider {
    return get_pgbackend()->get_is_recoverable_predicate();
  }
 protected:
-  OSDMapRef last_persisted_osdmap_ref;
+  epoch_t last_persisted_osdmap;


I don't understand how swapping these out helps us. If it's the latest persisted map we are certainly going to need the intervening maps when we start updating?

osd/PG: keep epoch, not map ref, of last osdmap for lsat persisted epoch No need to pin the map in memory!

I know the commit says that, but don't we need to refer to all the following maps?
...no, no we don't, because we've already done the processing and just haven't persisted that we've done so. Okay.

gregsfortytwo · 2018-01-17T02:10:16Z

src/osd/OSD.cc

@@ -6421,6 +6397,50 @@ void OSD::ms_fast_dispatch(Message *m)
    m->put();
    return;
  }
+
+  // peering event?
+  switch (m->get_type()) {


Why are none of the PG ops becoming proper OpRequests for tracking purposes?

gregsfortytwo · 2018-01-17T02:13:50Z

src/osd/OSD.cc

+      MOSDPeeringOp *pm = static_cast<MOSDPeeringOp*>(m);
+      return enqueue_peering_evt(
+	pm->get_spg(),
+	PGPeeringEventRef(pm->get_event()));


This appears to have lost the security check that our peer is actually an OSD, along with a few other subtler things I'm not sure we need (OSD::require_self_aliveness should be replaced by the PG::old_peering_msg check, but I didn't trace through the is_active() check that does).

gregsfortytwo · 2018-01-17T21:49:55Z

src/osd/PG.cc

+
+  // trim log when the pg is recovered
+  pg->calc_min_last_complete_ondisk();
+  return discard_event();


Now that we've replaced PG::handle_pg_trim() we need to actually remove that code.

gregsfortytwo · 2018-01-18T01:33:23Z

src/osd/OSD.cc

+    handle_command(static_cast<MMonCommand*>(m));
+    return;
+  case MSG_COMMAND:
+    handle_command(static_cast<MCommand*>(m));


This can immediately invoke con->send_message(). Embarrassingly, I've forgotten if we're allowed to do that inside of fast dispatch, but I don't think so?

it's safe, we do it all over the place

We definitely don't do it all over the place, and I'm quite sure we couldn't safely send messages out of fast_dispatch in the initial implementation, which was part of why we had all the elaborate tooling around scheduling map sharing to happen later on (struct send_map_on_destruct put a C_SendMap in the work queue in Jewel, for instance). It might have become safe since then (especially with the Connection-based functions or AsyncMessenger), but those races were subtle and unpleasant so we'll need to either avoid doing so or work through the locking again to prove it.

The only place we've ever done so that I can find is an error condition we wouldn't have tested: we return ENXIO on misdirected ops in-line. That in fact was something I kept around, but I think I just missed it.

(Really kicking myself for not documenting what was allowed here, since I thought we'd done such a good job dispatching it. But I think I just assumed sending messages fell under "long-held locks", and there were a bunch of cyclical deadlocks that involved a PG lock held while sending off messages while we were trying to dispatch a message to them...)

Hmm, yeah I remember it being weird, but I also can't remember why. The first case I remember reviewing or doing this is the fast dispatch for osd heartbeats which went in a while back.. see handle_osd_ping(), which will also send replies.

Stepping back, I don't see any reason why it shouldn't be possible. If there are remaining reasons (like loopback causing deadlock/recursion) then IMO we should fix that instead!

The global SimpleMessenger::lock is the obvious reason it would have caused trouble. The Connection-based interfaces are a lot safer, but still not guaranteed since they have to grab a bunch of extra locks if the pipe failed and needs to reconnect.

Handling reconnect means that guaranteeing a message dispatch is non-blocking is basically impossible unless we switch that model around completely; even if that doesn't cause a deadlock it's still distinctly undesirable — and it makes hard-to-identify deadlocks a lot more likely.

I'm not really sure I see a path that lets us generically guarantee that sending messages from fast_dispatch is safe. We could probably do it (by which I mean, it probably already works) if we restrict it to lossy connections. But I hate making use of interfaces which are that subtle and easy to break.

Fast dispatch works reasonably well because we made a simple interface: you're not allowed to do anything that might block. No matter how good we are, we can't send messages without doing something that might block. Therefore, we can't send messages in fast dispatch. It's the locking boundary that makes an interface like this between the OSD and Messenger possible.

I will note that in a seastar world this is much easier because if we have to do something blocking like establishing a new connection, it's easy to queue it to happen later. We just don't have that infrastructure right now.

And oh yeah, pipe replacement requires us to stop the previous Pipe's fast dispatch. There we go, deadlock.

Since we already have an easy model for queuing messages to go out not in the fast dispatch path, that seems a better idea right now than a lot of work that will destabilize our existing code and interfaces.

Oh bother.. I remember those deadlocks now. Yeesh.

Okay... so I think the primary value here is that it is funneling everything through the queue, and the message handlers are just generating queue events. I'll move these back to slow dispatch but leave the rest the same. I don't think that will make things any easier or harder in the eventual seastar transition. The hard part there is going to be handle_osd_map and the various bits of OSDService that have their own special locks (and aren't sharded).

That works too, and yes, I don't expect it to have any real impact on our future work!

...and yeah MSG_COMMAND looks like the only one, so no biggie!

gregsfortytwo · 2018-01-18T01:40:37Z

src/osd/OSD.h

@@ -1529,7 +1528,6 @@ class OSD : public Dispatcher,
    assert(osd_lock.is_locked());
    finished.splice(finished.end(), ls);
  }
-  void do_waiters();


If this machinery is dead, we need to remove the finished and take_waiters() members as well. But we're still pushing stuff on in OSD::activate_map(), from when OSD::handle_pg_create() pushes it on via require_same_or_newer_map(). So I don't think we can kill it quite yet.

yeah, teh legacy pg create handler is still using it!

So that means we still need to do_waiters() then.

gregsfortytwo · 2018-01-18T01:41:49Z

src/osd/OSD.cc

+    ShardData* sdata = shard_list[shard_index];
+    Mutex::Locker l(sdata->sdata_op_ordering_lock);
+    ShardData::pg_slot& slot = sdata->pg_slots[pgid];
+    slot.waiting_for_pg = true;


It's not wrong, but the ShardedOpWQ sure is grossly aware of what's going on with PGs at this point. :(

yes. the shardedwq becomes the complicated box that everything funnels through. my hope is that this will continue to be consolidated into an overall OSDShard-type concept (not just the queue), and ultimately be simpler to understand and replace in a seastar world than the hodge-podge of threads and locks we had before.

gregsfortytwo · 2018-01-18T01:47:40Z

src/messages/MOSDForceRecovery.h

+      return;
+    }
+    header.version = HEAD_VERSION;
+    header.compat_version = COMPAT_VERSION;


You're setting the HEAD/COMPAT_VERSION to their default values. I think you meant to set them to an old value if we don't have SERVER_MIMIC?

gregsfortytwo · 2018-01-18T01:48:28Z

src/messages/MOSDForceRecovery.h

+      decode(options, p);
+      for (auto pg : pgs) {
+	// note: this only works with replicated pools.  if a pre-mimic mon
+	// tries to force a mimic+ osd on an ec pool it will not work.


Sorry, I don't really understand how the forced recovery happens. Did it not previously work in EC pools at all? Or is this a regression?

previously it encoded pg_t, which the OSD had to go and look up in the pg_map. Fast dispatch needs a full spg_t to queue the event. For replicated pools spg_t::shard is NO_SHARD so it's easy, but for EC pools we don't know which shard to send it to. Since forced recovery is a rare operator event I'm opting to simply break it for the mid-upgrade old mon + new osd case.

Mmmm, that's probably okay but will need to include documentation. People occasionally get in to a lot of trouble during upgrades (or upgrade because of trouble!) and we need to make it clear that they can't do this.

...actually, we need to figure out a way to block this sensibly. Probably throw a clog error?

rzarzynski · 2018-01-18T18:57:17Z

src/osd/OSD.cc

+  PG *pg,
+  GenContext<ThreadPool::TPHandle&> *c)
+{
+  epoch_t e = get_osdmap()->get_epoch();


Maybe use OSDService::get_osdmap_epoch() instead? The main benefit would be no shared_ptr dance.

│ _ZNSt14__shared_countILN9__gnu_cxx12_Lock_policyE2EED4Ev(): │ // Does not throw if __r._M_get_use_count() == 0, caller must │ explicit __shared_count(const __weak_count<_Lp>& __r, std::no │ │ ~__shared_count() noexcept │ { │ if (_M_pi != nullptr) │ mov -0x68(%rbp),%rdi 1,59 │ mov -0x70(%rbp),%rax │ test %rdi,%rdi │ mov 0x10(%rax),%r13d 26,29 │ ┌──je 4ba45e <OSDService::queue_recovery_context(PG*, GenContext │ │ _M_pi->_M_release(); │ │→ callq std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2 │ │_ZN10OSDService22queue_recovery_contextEP2PGP10GenContextIRN10Threa │ │ epoch_t e = get_osdmap()->get_epoch(); │ │ enqueue_back( │ │ OpQueueItem( 6,37 │ 6e:└─→callq ceph_clock_now()

rzarzynski · 2018-01-18T19:04:30Z

src/osd/OSD.cc

+  enqueue_back(
+    OpQueueItem(
+      unique_ptr<OpQueueItem::OpQueueable>(
+	new PGRecoveryContext(pg->get_pgid(), c, e)),


std::make_unique<PGRecoveryContext>?

liewegas · 2018-02-23T16:58:21Z

@gregsfortytwo this is getting pretty close to mergeable; want to review the new bits? last time you looked only the first half of the commits were here. the second half kills the global pg_map, moves most of the ShardedOpWQ bits into a top-level OSDShard class, and reimplements the split handling (much simpler, yay!).

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

So that comment makes sense Signed-off-by: Sage Weil <sage@redhat.com>

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

This is only used for waiting. Signed-off-by: Sage Weil <sage@redhat.com>

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

Comment rules and take lock during init/shutdown (nice but not necessary). Signed-off-by: Sage Weil <sage@redhat.com>

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

This is cheaper and more explicit. Signed-off-by: Sage Weil <sage@redhat.com>

If the entry already exists, we don't want to construct the PGSlot and then throw it out. Signed-off-by: Sage Weil <sage@redhat.com>

Output as a proper array this time. Signed-off-by: Sage Weil <sage@redhat.com>

These are generally things to fix aspirationally someday or things that we'll need to clean up as part of the move to seastar. Signed-off-by: Sage Weil <sage@redhat.com>

liewegas · 2018-04-04T15:31:07Z

Clarified all of the FIXME comments

gregsfortytwo · 2018-04-04T15:41:50Z

Ugh, sorry, my brain's getting mushy around these.

…

On Wed, Apr 4, 2018 at 8:18 AM Sage Weil ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/osd/OSD.cc <#19973 (comment)>: > + } + if (!slot.waiting.empty()) { + if (osdmap->is_up_acting_osd_shard(pgid, osd->get_nodeid())) { + dout(20) << __func__ << " " << pgid << " maps to us, keeping" + << dendl; + ++p; + continue; + } + while (!slot.waiting.empty() && + slot.waiting.front().get_map_epoch() <= osdmap->get_epoch()) { + auto& qi = slot.waiting.front(); + dout(20) << __func__ << " " << pgid + << " waiting item " << qi + << " epoch " << qi.get_map_epoch() + << " <= " << osdmap->get_epoch() + << ", stale, dropping" << dendl; That second loop is discarding waiting items for this same pgid slot (slot.waiting is a deque iirc) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19973 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3cv-5iY1rMyZ_HvqkqtF0rFNNj5aIRks5tlOMUgaJpZM4RgfsU> .

… is created" This reverts commit 38319f8. I want to reproduce the race to see if there is a better way to avoid it. This breaks tests that set pg_num, and generally makes interacting with the cluster awkward for anything non-human. # Conflicts: # src/mon/CreatingPGs.h Signed-off-by: Sage Weil <sage@redhat.com>

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

For the legacy create path, we pregenerate the history and instantiate the PG at the current epoch. Signed-off-by: Sage Weil <sage@redhat.com>

liewegas · 2018-04-06T17:16:27Z

http://pulpito.ceph.com/sage-2018-04-06_13:59:16-rados-wip-sage2-testing-2018-04-06-0721-distro-basic-smithi/ looks good!

liewegas added cleanup core wip-sage2-testing labels Jan 16, 2018

liewegas requested a review from gregsfortytwo January 16, 2018 22:04

liewegas force-pushed the wip-peering-fast-dispatch branch from a0c1e49 to e243ba9 Compare January 17, 2018 22:24

gregsfortytwo requested changes Jan 18, 2018

View reviewed changes

rzarzynski reviewed Jan 18, 2018

View reviewed changes

liewegas force-pushed the wip-peering-fast-dispatch branch 2 times, most recently from 7db1a13 to 7cc290b Compare January 18, 2018 22:29

liewegas force-pushed the wip-peering-fast-dispatch branch 4 times, most recently from 1fbea7c to 080e43e Compare January 29, 2018 22:54

liewegas force-pushed the wip-peering-fast-dispatch branch 3 times, most recently from d639f1e to 32ce304 Compare February 12, 2018 21:51

liewegas mentioned this pull request Feb 16, 2018

osd, common: remove 2 memory indirection layers in ShardedOpWQ #20228

Closed

liewegas force-pushed the wip-peering-fast-dispatch branch 5 times, most recently from 2b57d4f to 43336f7 Compare February 22, 2018 03:19

liewegas mentioned this pull request Feb 23, 2018

osd,mon: implement PG merging #20469

Merged

9 tasks

liewegas force-pushed the wip-peering-fast-dispatch branch from d23dee6 to af24310 Compare February 23, 2018 16:56

liewegas force-pushed the wip-peering-fast-dispatch branch 3 times, most recently from facf139 to 79e753d Compare February 26, 2018 20:51

liewegas changed the title ~~osd: fast dispatch of peering events~~ osd: fast dispatch of peering events and pg_map + osd sharded wq refactor Feb 27, 2018

liewegas added 17 commits April 4, 2018 08:26

osd: constify arg to prime_splits

db35bbd

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

osd: constify arg for handle_pg_create_info, maybe_wait_for_max_pg

0838174

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

osd: fix 'stale' message

dc66a05

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

osd: fix old wake_pg_waiters references

bfbf204

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

osd: accessors for num_pgs

a01d2df

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

osd: reduce debug level for pg epoch min

77179eb

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

osd: move sdata_op_ordering_lock declaration

a25d6fd

So that comment makes sense Signed-off-by: Sage Weil <sage@redhat.com>

osd: rename OSDPGShard::osdmap -> shard_osdmap

0c2970f

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

osd: OSDShard: sdata_lock -> sdata_wait_lock

20f6a86

This is only used for waiting. Signed-off-by: Sage Weil <sage@redhat.com>

osd: OSDShard: sdata_op_ordering_lock -> shard_lock

e05ddde

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

osd: drop obsolete Pred

073dc92

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

PendingReleaseNotes: mons before osds to avoid force-recovery issue

3ecfa25

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

PendingReleaseNotes: not pg_num adjustments during pool create

68c505d

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

osd: clean up osdmap_lock

0724072

Comment rules and take lock during init/shutdown (nice but not necessary). Signed-off-by: Sage Weil <sage@redhat.com>

os/ObjectStore: add has_contexts()

b85a24a

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

osd: check for rctx contexts instead of adding nop() to transaction

9dbb9f9

This is cheaper and more explicit. Signed-off-by: Sage Weil <sage@redhat.com>

osd: avoid OSDShardPGSlot allocation in fast path

d14221d

If the entry already exists, we don't want to construct the PGSlot and then throw it out. Signed-off-by: Sage Weil <sage@redhat.com>

liewegas force-pushed the wip-peering-fast-dispatch branch from 116ea62 to d14221d Compare April 4, 2018 13:27

liewegas added 2 commits April 4, 2018 10:26

osd: reimplement get_mapped_pools

1f4f8ce

Output as a proper array this time. Signed-off-by: Sage Weil <sage@redhat.com>

osd: clarify FIXME comments

5b9c0c1

These are generally things to fix aspirationally someday or things that we'll need to clean up as part of the move to seastar. Signed-off-by: Sage Weil <sage@redhat.com>

rzarzynski mentioned this pull request Apr 5, 2018

common: optimize OpTracker #20702

Closed

liewegas added 3 commits April 5, 2018 13:04

mon/OSDMonitor: keep sending legacy create messages for now

b626b4d

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

mon/OSDMonitor: fix warnings

4cb194e

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

osd: legacy pg_create creates pg in current epoch

922a855

For the legacy create path, we pregenerate the history and instantiate the PG at the current epoch. Signed-off-by: Sage Weil <sage@redhat.com>

gregsfortytwo approved these changes Apr 6, 2018

View reviewed changes

gregsfortytwo merged commit 6d2e4c9 into ceph:master Apr 6, 2018

osd: fast dispatch of peering events and pg_map + osd sharded wq refactor #19973

osd: fast dispatch of peering events and pg_map + osd sharded wq refactor #19973

Conversation

liewegas commented Jan 16, 2018 • edited

liewegas commented Jan 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rzarzynski Jan 18, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liewegas commented Feb 23, 2018

liewegas commented Apr 4, 2018

gregsfortytwo commented Apr 4, 2018 via email

liewegas commented Apr 6, 2018

liewegas commented Jan 16, 2018 •

edited

rzarzynski Jan 18, 2018 •

edited