Earlier  
Posted Nick Remark
#openstack-nova - 2019-05-10
19:57:17 mriedem not sure if you're asserting provider usage or consumer usage
19:57:39 mriedem i'm assuming the former
19:57:58 ganso mriedem: yup, doing that now. Basically re-coding the test to perform checks as it was done in queens
19:58:06 ganso mriedem: yea provider usage
20:13:21 openstackgerrit Rodrigo Barbieri proposed openstack/nova stable/queens: [DEBUG] Add functional confirm_migration_error test https://review.opendev.org/658136
20:22:17 jaypipes zzzeek: pong
20:22:24 zzzeek hey
20:22:28 jaypipes heya :)
20:22:55 zzzeek jaypipes: time warp back to http://www.joinfu.com/2015/01/understanding-reservations-concurrency-locking-in-nova/
20:24:02 zzzeek so given that galera can have a little bit of read latency: https://www.percona.com/blog/2013/03/03/investigating-replication-latency-in-percona-xtradb-cluster/ can that impact an UPDATE that says, "UPDATE ... SET x='bar' WHERE x = 'foo'" ? if two transactions both update, but write latency means transaction 2 still sees "foo" ?
20:24:34 zzzeek OR, do the UPDATE statements in a write-set get replayed such that they are serialized and it will detect this ?
20:25:00 zzzeek AND, if the latter, what if the two UPDATE statements are changing "x" to the *same* value? this is ultimately a nova question
20:25:28 zzzeek b.c. the issue is observed in the instance shelving logic
20:35:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add cross-cell resize policy rule and enable in API https://review.opendev.org/638269
20:35:21 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Enable cross-cell resize in the nova-multi-cell job https://review.opendev.org/656656
20:35:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Support cross-cell moves in external_instance_event https://review.opendev.org/658478
20:36:43 mriedem zzzeek: a la this bug? https://bugs.launchpad.net/nova/+bug/1821373
20:36:44 openstack Launchpad bug 1821373 in OpenStack Compute (nova) "Most instance actions can be called concurrently" [Undecided,New]
20:37:00 zzzeek mriedem: yes
20:37:23 zzzeek mriedem: mdbooth_ has updated me that his proposed solution won't work
20:39:22 mriedem i think he mentioned something to that effect in irc awhile back for this bug but i don't remember what or why
20:44:06 mriedem gotta run
20:48:57 cdent so many things
20:51:28 cdent zzzeek: since you're sort of around, I wanted to warn you that at some point we might be hassling you for some advice on some performance improvements in placement.
20:51:47 cdent but not now. later.
20:51:54 zzzeek cdent: that should be good since placement looks to be straightforward
20:52:03 cdent it _was_
20:52:53 cdent but the nested stuff being added this cycle are going to be mind bending
20:55:10 zzzeek cdent: what kind of nesting
21:07:09 edleafe zzzeek: representing compute nodes containing NUMA nodes containing memory, for example
21:07:18 cdent zzzeek: sorry was away. the nesting is the "nested providers" concept. The new features are described (in brief) at https://storyboard.openstack.org/#!/story/2005575
21:07:54 cdent zzzeek edleafe has done some interesting work to switching a graph database (which is a better fit for some of these things) but we're in the standard position of "we've got this other stuff already"
21:09:00 zzzeek cdent: so for now are you looking at...adjacency list schema ?
21:10:11 cdent zzzeek: no explicit decisions yet. the linked email thread has some ideas. But the lack of decisions is why I was saying we'll probably want some input, but not quite yet. It's more of a heads up for a later discussion rather than a now discussion
21:11:25 cdent on top of that: solutions for the existing features are going to need some work to scale into the 100s of thousands of resource providers
21:11:42 cdent but we need to do some accurate measuring first
21:19:08 zzzeek edleafe is dying to get off SQL :P
21:19:09 zzzeek :)
21:19:31 edleafe zzzeek: Just for this case.
21:19:43 zzzeek edleafe: yeah graph DBs are awesome
21:19:53 zzzeek edleafe: huge crazy new dependencies not as much :)
21:20:04 edleafe zzzeek: I got nested providers working in a couple of days at the summit last week
21:20:06 zzzeek edleafe: might be one of htose cases where you have multiple backends
21:20:26 edleafe I just use Neo4j in docker :)
21:22:14 zzzeek edleafe: we still have to have RPMs that build it out, we have memory reuirements that java VM adds some weight towards, etc
21:22:24 zzzeek edleafe: by "we" I mean red hat
21:23:55 edleafe zzzeek: Sure, I understand all that. I just want to show it working, and solving the problems that we've spent years trying to solve using SQL. If that gets done and enough people feel that this is the way to go, then we can worry about what is needed for build/deploy
21:24:37 zzzeek edleafe: for small graphs, I use adjacency list. if you need a huge deep graph every time, then that won't work
21:25:18 jaypipes zzzeek: apologies, having net issues... reading back
21:25:46 cdent I think our model is many trees, each <= 7 levels deep, not very broad
21:26:16 edleafe cdent: for nested, agree. Shared providers is the opposite
21:27:14 cdent depends on how we model the shared association. in a graph db, yes
21:28:23 cdent I tend to like shared as group, not tree
21:35:16 openstackgerrit Eric Fried proposed openstack/os-resource-classes master: Propose ACCELERATOR_{FPGA|GPU} resource classes https://review.opendev.org/657464
21:36:17 edleafe cdent: I was just commenting on the trees reference. In the graph, sharing is just another relation. With Many:1 sharing, it kind of looks like a flower, not a tree: https://bit.ly/2VUI52U
21:38:30 jaypipes zzzeek: set global wsrep_causal_reads=1; <-- do that if you want synchronous replication behaviour.
21:38:59 zzzeek jaypipes: OK so you can confirm that comapre and swap can fail for multimaster if that's not set ?
21:39:25 zzzeek jaypipes: this might be what's needed for https://bugs.launchpad.net/nova/+bug/1821373
21:39:26 openstack Launchpad bug 1821373 in OpenStack Compute (nova) "Most instance actions can be called concurrently" [Undecided,New]
21:43:35 jaypipes zzzeek: I'm not 100% sure, but I believe so. the issue, however, is that the compare-and-swap technique should be done to increment a field and check that field value is at a previous read-view in the WHERE clause. in other words, doing things like UPDATE instances SET status = 'active' WHERE status IN (<list of statuses>) AND instances.uuid = ? is inherently not as safe/efficient/retryable-with-confidence as UPDATE instances SET generation =
21:43:35 jaypipes ? + 1 WHERE generation = ? AND uuid = ?
21:44:22 jaypipes zzzeek: that's the fundamental problem IMHO with the whole "check my instance status is in these list of states and set the status to X" checks that nova does.
21:45:12 zzzeek jaypipes: if the UPDATE included a unique version or timestamp of some kind does that help? if galera sees two UPDATE statements setting it to a different value ?
21:48:41 jaypipes zzzeek: maybe? :) galera already sends around essentially a generation for the innodb records affected by a transaction writeset, AFAIK. I just think doing the "compare" part of the compare-and-swap functionality with a "loose match" like "status IN <....>" isn't as good as comparator that was specifically designed to inform the caller that "yes, someone else changed this record since you last read a view of it". hope that makes sense.
21:50:13 jaypipes zzzeek: this is why, in placement land, we always do the compare-and-swap using the `UPDATE tbl SET generation = ($LAST_READ_GENERATION + 1) WHERE pk = $PK AND generation = $LAST_READ_GENERATION` strategy
21:50:15 zzzeek jaypipes: the specific case in that issue we are looking for an exisitng status of NULL
21:51:41 jaypipes zzzeek: yeah, but AFAIK, the code you're describing isn't really a compare-and-swap. it's more just a "hey, check that I'm not, say, in the process of deleting this instance when I try to unshelve it"
21:51:50 zzzeek jaypipes: well yes I was saying, it's more reliable if we are setting it to a new value, however, if two transactions on different nodes hit it at the same time they will see the same LAST_READ_GENERATION value
21:52:25 jaypipes zzzeek: yes, they will.
21:52:38 jaypipes zzzeek: and if both attempt to update, one will fail of course.
21:53:30 zzzeek jaypipes: one fails because of the SET clasue specifically ?
21:54:17 jaypipes zzzeek: no, one will fail because WHERE generation = $LAST_READ_GENERATION will fail.
21:54:22 jaypipes to return a row.
21:54:55 jaypipes zzzeek: so, the SQL won't fail, per-se, it's just the transaction will return 0 rows affected.
21:55:09 jaypipes zzzeek: which is the thing we look for to trigger a rollback of the entire transaction.
21:55:18 zzzeek jaypipes: but what if one UPDATE processes, sends out the writeset which includes the new value, however the other node gets an UPDATE, due to replcation latency it also sees the same value, also emits an UPDATE, no failure
21:55:28 zzzeek but updates the row
21:55:40 zzzeek beause the new vaule wasn't there yet b.c. no wsrep_causal_reads
21:56:05 zzzeek jaypipes: this gets into, I have no idea how the galera writeset certification works
21:56:32 zzzeek jaypipes: I would think that certifiaction should be, transaction modified this row, this other transacvtion is modifying the same generation of that row, so it fails
21:56:39 zzzeek which means this issue is non-existent
21:56:43 zzzeek e.g. mvcc generation
21:57:43 jaypipes that is essentially how it works, yes.
21:57:50 jaypipes https://github.com/openstack/placement/blob/master/placement/objects/resource_provider.py#L960
21:58:52 zzzeek jaypipes: OK. So, if the SHELVE thing is looking explicitly for NULL and changes to SHELVING as described in the launchpad, no failure ? how does it fail ?
21:58:56 jaypipes the wsrep_causal_reads is about reads only. writes that attempt to update the same record when a diff trx changed that record are never allowed.
21:59:07 zzzeek jaypipes: right that's what I sort of thought
21:59:26 jaypipes zzzeek: apooogies, I haven't read the bug report yet.
21:59:32 jaypipes lemme do that now. one minute.
22:00:40 jaypipes zzzeek: I would take issue with mdbooth_'s statement "This is intended to act as a robust gate against 2 instance actions happening concurrently." :)
22:00:48 jaypipes it's not a robust gate at all.
22:01:01 jaypipes it's a super coarse-grained check
22:01:15 zzzeek jaypipes: OK he is stumped on this and I dont really know the details of this system, I just wrote the UPDATE statement four years ago
22:01:23 zzzeek do you have anything you can add to that launchapd?
22:01:42 jaypipes in fact, it's not a gate at all. it's nothing more than a very simple sanity check that exists outside of any transactional context AFAIK
22:02:03 jaypipes zzzeek: yeah, I will add a note to it.
22:02:03 zzzeek jaypipes: really? it doesnt seem that way to me, I assume this is on enginefacade and there should be a tx
22:02:32 jaypipes zzzeek: one would assume that. I have no real way of verifying it's in the same trx though.
22:04:55 zzzeek thanks for the chat jaypipes
22:33:45 openstackgerrit Eric Fried proposed openstack/os-resource-classes master: Propose ACCELERATOR_{FPGA|GPU} resource classes https://review.opendev.org/657464

Earlier   Later