| *** thinrichs1 has joined #congress | 00:01 | |
| *** absubram has quit IRC | 00:20 | |
| ekcs | thinrichs1: so if two rules inserted by diff people cause recursion, my concern is it’d be very helpful for whoever inserted the last rule to know that right away. | 01:02 |
|---|---|---|
| ekcs | thinrichs1: instead of thinking there is no problem and only seeing it down the road when an evaluation happens to hit that rule. | 01:02 |
| thinrichs1 | Sure ideally. | 01:03 |
| ekcs | part of the issue is it’s hard to let people know a problem occurred. especially if it’s being used by a program over API. | 01:04 |
| ekcs | are you tihnking evaluation fails when a table is queried? | 01:04 |
| thinrichs1 | This is a problem where 2 people create rules that cause recursion basically simultaneously (since sync won't have run). They're both defining a *new* helper table say p and q. | 01:05 |
| thinrichs1 | ekcs: we could have eval totally fail if it finds recursion (throw an exception or something) | 01:06 |
| thinrichs1 | Still trying to figure out a practical way for 2 people to accidentally cause recursion. | 01:06 |
| ekcs | yes basically simultaniously. but not necessarily new helper tables. could be new rules for existing helper tables. | 01:07 |
| ekcs | kind of a silly example is this, but you can see how it can happen in a more realistic way. | 01:08 |
| ekcs | there is a vm table and a server table. | 01:08 |
| ekcs | both defined tables. | 01:08 |
| ekcs | one person says oh all vms should be servers so adds a rule to say that. | 01:08 |
| ekcs | another says oh all servers should be vms so adds a rule to say that. | 01:08 |
| ekcs | and in addition they may write other rules using these tables and expecting those rules to be working, say to fire reactive enforcement actions. | 01:09 |
| ekcs | in reality it should still be very very unlikely. | 01:10 |
| ekcs | because of the simultaneity requirement. | 01:11 |
| thinrichs1 | In the vm/server example, they're both already defined, right? But both people see that the VMs and servers are not identical. | 01:12 |
| thinrichs1 | So either there are VMs that aren't servers or servers that aren't VMs or both. | 01:12 |
| ekcs | right. they may be defined based on different data sources and rules. | 01:12 |
| thinrichs1 | If both are missing elements, both people would know not to do vm :- server and server :- vm b/c of recursion. | 01:13 |
| thinrichs1 | Otherwise, there'd be 1 that's missing stuff, so you'd add the 1 rule. And both people would add the same rule. | 01:13 |
| thinrichs1 | Seems we're agreed that this is very unlikely. | 01:14 |
| thinrichs1 | So the question is what to do about it. | 01:14 |
| thinrichs1 | Do we pay a DB-locking + policy-sync on every rule insert to catch that rare case? | 01:14 |
| thinrichs1 | Or do we allow that rare case to make it into the DB and then error out when doing eval? (Or maybe some other option.) | 01:15 |
| ekcs | both may be missing elements. person A decides to make server the complete on and do things based on that. simultaneously person B may becode to made vm the complete one and do things based on that. | 01:15 |
| ekcs | there are three main options I think | 01:15 |
| ekcs | 1. lock and sync before each write. | 01:15 |
| ekcs | 2. allow inserts and let eval fail or disable rule later. | 01:16 |
| ekcs | 3. 2 phase commit style thing where rules are added to staging, check for no recursion (in a way that may be cheaper than sync, especially if the db supports recursion), then proceed if clear. | 01:17 |
| ekcs | 3 is complicated to implement and to get right. but does not require locking. | 01:18 |
| thinrichs1 | Oooh—that's right. We have a mechanism to disable rules. We'd probably still want to stop the infinite-loop in the eval. Maybe we can periodically check for recursion and disable bad rules. We need a way to give those kind of async error messages to the user anyway. | 01:18 |
| ekcs | yea it’d be great if we have a way. | 01:18 |
| thinrichs1 | Maybe we do whatever we have implemented. Policy rules don't change that often, so even if it's expensive it should be fine. What happens though if we can't sync (for an extended period of time like from a network partition)? | 01:20 |
| ekcs | right now CUD stop working if you can’t reach the DB. | 01:21 |
| ekcs | we can work to relax that if needed. | 01:23 |
| ekcs | i guess we’d do some kind of manual rejoin that can show errors in the rare case that diff nodes have defined conflicting rules causing recursion. | 01:23 |
| ekcs | but in general right now things just fail (with most things) if DB cannot be reached. | 01:24 |
| thinrichs1 | That sounds good. | 01:24 |
| ekcs | how important is it to operate without DB? | 01:25 |
| thinrichs1 | Not important. | 01:25 |
| ekcs | ok then. I don’t like the locking, but then again many other things the way they are don’t work with multi-master DB either. it’d be another undertaking to support that anyway. no need to do that prematurely. | 01:27 |
| thinrichs1 | Yep. The locking seems fine for now. If it ends up being too expensive, we can revisit later. | 01:27 |
| ekcs | alrite then! thanks! | 01:28 |
| thinrichs1 | In other news… looks like the grammar is parsing IDs with hyphens.. | 01:28 |
| thinrichs1 | So the check is running. | 01:29 |
| ekcs | hope it’s a grammar bug rather than antlr bug. | 01:29 |
| ekcs | but i didn’t see how from the grammar. | 01:29 |
| thinrichs1 | Test is <tablename>() :- true() | 01:34 |
| thinrichs1 | When giving it invalid-table-name, parses as … | 01:34 |
| thinrichs1 | rule: invalid-() | 01:34 |
| thinrichs1 | rule: table-() | 01:34 |
| thinrichs1 | rule: name() :- true() | 01:34 |
| thinrichs1 | (without the rule: prefix) | 01:35 |
| thinrichs1 | Not a parser bug. A bug in the check. Simple fix. | 01:35 |
| ekcs | oh so that’s how it’s supposed to parse? | 01:35 |
| ekcs | oh i see. | 01:36 |
| thinrichs1 | If I pull that check out of the agnostic.py, where do you think I should put it so we can use it for both datasources and policies? | 01:36 |
| thinrichs1 | Those - are from the fake modals. | 01:36 |
| ekcs | I was thinking congress/utils | 01:37 |
| thinrichs1 | Sounds good. | 01:38 |
| ekcs | so it’s similar to how invalid+table+name would parse? | 01:38 |
| thinrichs1 | I'd imagine that would parse too. | 01:47 |
| thinrichs1 | congress.utils is too low level. congress.exceptions imports congress.utils, and congress.datalog.compile imports congress.exceptions. The test uses congress.datalog.compile. | 01:48 |
| thinrichs1 | I could just add it to congress.datalog.compile, which is a natural place. | 01:48 |
| ekcs | sounds good. | 01:53 |
| openstackgerrit | Tim Hinrichs proposed openstack/congress: Fix check that policy-names are valid IDs in the grammar https://review.openstack.org/416814 | 02:36 |
| *** thinrichs1 has quit IRC | 02:38 | |
| openstackgerrit | Merged openstack/congress: Updated from global requirements https://review.openstack.org/415957 | 03:04 |
| *** ekcs has quit IRC | 03:08 | |
| *** masahito has quit IRC | 04:24 | |
| *** ekcs has joined #congress | 06:17 | |
| *** ekcs has quit IRC | 06:24 | |
| openstackgerrit | Merged openstack/congress: Add get_item call in datasource_model https://review.openstack.org/412303 | 07:39 |
| *** openstackgerrit has quit IRC | 07:50 | |
| *** openstackstatus has quit IRC | 10:13 | |
| *** openstack has joined #congress | 10:15 | |
| *** absubram has joined #congress | 14:58 | |
| *** thinrichs has joined #congress | 15:53 | |
| *** ekcs has joined #congress | 15:54 | |
| *** openstackgerrit has joined #congress | 16:26 | |
| openstackgerrit | Tim Hinrichs proposed openstack/congress: Add namespace 'builtin' for builtins https://review.openstack.org/399124 | 16:26 |
| openstackgerrit | Tim Hinrichs proposed openstack/congress: Fix check that policy-names are valid IDs in the grammar https://review.openstack.org/416814 | 16:35 |
| *** ekcs has quit IRC | 18:35 | |
| *** thinrichs has quit IRC | 20:01 | |
| *** thinrichs has joined #congress | 21:18 | |
| openstackgerrit | Eric K proposed openstack/congress: WIP: Lock table and sync rule before each rule insert https://review.openstack.org/416777 | 21:23 |
| *** ekcs has joined #congress | 23:02 | |
| *** ekcs has quit IRC | 23:11 | |
| *** ekcs has joined #congress | 23:19 | |
| *** absubram has quit IRC | 23:28 | |
| *** ekcs has quit IRC | 23:48 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!