| *** amcrn has quit IRC | 00:16 | |
| *** EricGonczer_ has joined #openstack-dns | 00:18 | |
| *** vinod1 has joined #openstack-dns | 00:21 | |
| openstackgerrit | Vinod Mangalpally proposed a change to openstack/designate: Call _handle_query_error correctly https://review.openstack.org/119229 | 00:36 |
|---|---|---|
| *** vinod1 has quit IRC | 00:37 | |
| *** rmoe has quit IRC | 00:37 | |
| *** darshan104 has quit IRC | 00:42 | |
| *** darshan104 has joined #openstack-dns | 00:43 | |
| *** darshan104 has quit IRC | 00:47 | |
| *** rmoe has joined #openstack-dns | 00:53 | |
| *** EricGonczer_ has quit IRC | 00:58 | |
| *** EricGonczer_ has joined #openstack-dns | 01:00 | |
| *** EricGonczer_ has quit IRC | 01:11 | |
| *** lpmulligan has quit IRC | 01:22 | |
| *** EricGonczer_ has joined #openstack-dns | 01:33 | |
| *** nosnos has joined #openstack-dns | 01:49 | |
| *** rossk has quit IRC | 01:51 | |
| *** rossk has joined #openstack-dns | 01:52 | |
| *** rossk has quit IRC | 01:57 | |
| *** eankutse has joined #openstack-dns | 01:59 | |
| *** eankutse has quit IRC | 02:02 | |
| *** eankutse has joined #openstack-dns | 02:02 | |
| *** rossk has joined #openstack-dns | 02:15 | |
| *** eankutse has quit IRC | 02:18 | |
| *** rossk has quit IRC | 02:22 | |
| *** EricGonczer_ has quit IRC | 02:25 | |
| *** richm has quit IRC | 02:46 | |
| *** EricGonczer_ has joined #openstack-dns | 03:05 | |
| *** EricGonczer_ has quit IRC | 03:08 | |
| *** EricGonczer_ has joined #openstack-dns | 03:08 | |
| *** EricGonczer_ has quit IRC | 03:10 | |
| *** rjrjr has quit IRC | 03:29 | |
| *** vinod1 has joined #openstack-dns | 03:32 | |
| *** vinod1 has quit IRC | 04:23 | |
| *** ttrumm has joined #openstack-dns | 05:37 | |
| *** ttrumm_ has joined #openstack-dns | 05:38 | |
| *** ttrumm has quit IRC | 05:41 | |
| *** k4n0 has joined #openstack-dns | 06:25 | |
| *** ttrumm has joined #openstack-dns | 06:35 | |
| *** ttrumm_ has quit IRC | 06:38 | |
| *** fouxm_ is now known as fouxm | 07:19 | |
| *** darshan104 has joined #openstack-dns | 08:42 | |
| *** k4n0 has quit IRC | 09:16 | |
| *** darshan104 has quit IRC | 09:39 | |
| *** darshan104 has joined #openstack-dns | 09:40 | |
| *** darshan104 has quit IRC | 09:44 | |
| *** darshan104 has joined #openstack-dns | 09:50 | |
| *** darshan104 has quit IRC | 09:54 | |
| *** vinod1 has joined #openstack-dns | 10:02 | |
| *** vinod1 has quit IRC | 10:43 | |
| *** ttrumm_ has joined #openstack-dns | 10:47 | |
| *** ttrumm has quit IRC | 10:49 | |
| *** darshan104 has joined #openstack-dns | 10:51 | |
| *** darshan104 has quit IRC | 10:55 | |
| *** darshan104 has joined #openstack-dns | 11:51 | |
| *** k4n0 has joined #openstack-dns | 11:55 | |
| *** darshan104 has quit IRC | 11:56 | |
| *** openstackgerrit has quit IRC | 12:01 | |
| *** openstackgerrit has joined #openstack-dns | 12:02 | |
| *** ChanServ sets mode: +v openstackgerrit | 12:02 | |
| *** mwagner_lap has joined #openstack-dns | 12:10 | |
| *** nosnos has quit IRC | 12:19 | |
| *** nosnos has joined #openstack-dns | 12:20 | |
| *** nosnos has quit IRC | 12:24 | |
| *** bauruine has quit IRC | 12:28 | |
| *** bauruine has joined #openstack-dns | 12:34 | |
| *** vinod1 has joined #openstack-dns | 12:44 | |
| *** darshan104 has joined #openstack-dns | 12:54 | |
| *** darshan104 has quit IRC | 12:58 | |
| *** richm has joined #openstack-dns | 13:10 | |
| *** eankutse has joined #openstack-dns | 13:15 | |
| *** vinod1 has quit IRC | 13:18 | |
| *** ttrumm_ has quit IRC | 13:29 | |
| *** ttrumm has joined #openstack-dns | 13:29 | |
| *** ttrumm has quit IRC | 13:42 | |
| *** darshan104 has joined #openstack-dns | 13:55 | |
| *** darshan104 has quit IRC | 13:59 | |
| *** EricGonczer_ has joined #openstack-dns | 14:21 | |
| *** timsim has joined #openstack-dns | 14:28 | |
| *** vinod1 has joined #openstack-dns | 14:28 | |
| *** paul_glass has joined #openstack-dns | 14:30 | |
| *** paul_glass has quit IRC | 14:30 | |
| *** paul_glass has joined #openstack-dns | 14:30 | |
| *** jmcbride has joined #openstack-dns | 14:33 | |
| openstackgerrit | A change was merged to openstack/designate: Updated from global requirements https://review.openstack.org/119139 | 14:41 |
| *** ttrumm has joined #openstack-dns | 14:48 | |
| *** ttrumm has quit IRC | 14:49 | |
| *** rmoe has quit IRC | 14:53 | |
| *** k4n0 has quit IRC | 14:57 | |
| openstackgerrit | Vinod Mangalpally proposed a change to openstack/designate: Delete SOA records correctly on a downgrade https://review.openstack.org/119393 | 15:01 |
| *** paul_glass has quit IRC | 15:02 | |
| *** eankutse1 has joined #openstack-dns | 15:03 | |
| *** EricGonc_ has joined #openstack-dns | 15:03 | |
| *** eankutse has quit IRC | 15:03 | |
| *** EricGonczer_ has quit IRC | 15:04 | |
| *** paul_glass has joined #openstack-dns | 15:20 | |
| *** darshan104 has joined #openstack-dns | 15:28 | |
| *** darshan104 has quit IRC | 15:33 | |
| *** eankutse1 has quit IRC | 15:37 | |
| *** eankutse has joined #openstack-dns | 15:37 | |
| vinod1 | musie:/kiall: Had a question on bug 1365699 | 15:45 |
| uvirtbot | Launchpad bug 1365699 in designate "We could end up with multiple SOA recordsets in the recordset table" [High,Confirmed] https://launchpad.net/bugs/1365699 | 15:45 |
| vinod1 | When a domain is deleted why do we mark it as "deleted" rather than delete it? | 15:45 |
| Kiall | vinod1: for metering and billing purposes.. We need to know who owned a domain at a given time in order to take things like query aggregations and pass the bill onto the customer | 15:47 |
| *** darshan104 has joined #openstack-dns | 15:47 | |
| Kiall | it also for a little bit of safety against users rm -rf'ing all there data ;) | 15:48 |
| vinod1 | So having the ondelete here - https://github.com/openstack/designate/blob/master/designate/storage/impl_sqlalchemy/tables.py#L125 | 15:48 |
| vinod1 | does not do anything then | 15:48 |
| Kiall | The ondelete = CASCADE will be triggered when the domain is actually fully deleted.. Which is something we do periodically out of band | 15:50 |
| vinod1 | Kiall, so on deleting a domain, do we want to mark the corresponding recordsets and records too as deleted or do we want to actually delete the recordsets and records? | 15:50 |
| mugsie | vinod1: mark them the same way | 15:51 |
| Kiall | e.g. DELETE FROM `domains` WHERE `domains`.`deleted_at` > "1 Month Ago"; | 15:51 |
| mugsie | in the storage layer we try not to actually delete anything | 15:51 |
| Kiall | vinod1: there's been no need to do that before, as soft deleting the domain effectively get's rid of them | 15:51 |
| Kiall | vinod1: for mDNS, I think the right answer is to do a.. | 15:51 |
| *** darshan104 has quit IRC | 15:51 | |
| mugsie | but, with the current _delete() call in the current layer, it might be difficult | 15:52 |
| mugsie | (it takes a table, and an id) | 15:52 |
| mugsie | so that may need to be changed to take a qriterion as well | 15:52 |
| Kiall | SELECT `recordsets`.* FROM `recordsets` LEFT JOIN `domains` ON `recordsets`.`domain_id` = `domains`.`id` WHERE `recordsets`.`type` = 'SOA' AND `recordsets`.`name` = 'example.com.' AND `domains`.`deleted` = '0'; | 15:53 |
| *** eankutse has quit IRC | 15:53 | |
| mugsie | Kiall: we should be 'deleteing' the recordsets db entry as well tbh | 15:54 |
| mugsie | not patching the end result in mDNS | 15:54 |
| Kiall | I don't think so... | 15:54 |
| mugsie | its in consistant data | 15:54 |
| Kiall | And it becomes ambiguous data if we also marked the RRSet's as deleted ;) | 15:55 |
| mugsie | how is it ambigiuos? | 15:55 |
| Kiall | Lets say we go to restore a deleted zone, which recordsets should we restore? Some may have been deleted months ago. | 15:55 |
| mugsie | this rrset is deleted | 15:55 |
| mugsie | we store the delted date as part of the soft delete mixin | 15:55 |
| mugsie | so, thats kind of invalid | 15:56 |
| mugsie | it also mean that when a clean needs to happen - you have now idea if the rrsets are still active | 15:56 |
| Kiall | Humm, what about a RRSet that's deleted moments before the users bad script rm -rf's the whole zone? The timestamp may be the same | 15:57 |
| mugsie | that is a 0.1% use case | 15:57 |
| mugsie | that doesnt out wiegh storing corrct data | 15:57 |
| Kiall | The correct data is there, it just needs to be queried correctly ;) | 15:58 |
| mugsie | and tbf, a customer would just be happy you could get any data back, and if it had an extra record, they should verify it anyway | 15:58 |
| mugsie | its not correct data - and it has the result for the mDNS single record lookup of 2x the db calls needed | 15:59 |
| mugsie | vs doing it properly | 15:59 |
| mugsie | for a tiny use case | 15:59 |
| Kiall | Well .. 1 DB call, with a JOIN.. | 16:00 |
| Kiall | Anyway - Regardless. We're in feature freeze.. and this is a bug in juno | 16:00 |
| Kiall | Implementing soft-delete on the recordsets and records table is not something that's going to happen to Juno | 16:00 |
| mugsie | we have soft delte on those tables | 16:01 |
| mugsie | its a matter of calling it | 16:01 |
| mugsie | not implementing it | 16:01 |
| Kiall | No, we don't... | 16:01 |
| Kiall | https://github.com/openstack/designate/blob/master/designate/storage/impl_sqlalchemy/tables.py | 16:01 |
| *** rmoe has joined #openstack-dns | 16:01 | |
| Kiall | recordsets and records don't have any of the necessary columns etc | 16:01 |
| mugsie | so your point about data recovery is totally invalid then..... | 16:02 |
| Kiall | mugsie: No, when a user accidentally presses the wrong "Delete" button in horizon, and the whole zone is deleted, the data is still there | 16:03 |
| mugsie | anyway - what ever - fix for 1365699 is for mDNS to fix the query (bug part 2 in my comment) | 16:04 |
| mugsie | i'll file a bug for the other bit now, for kilo | 16:04 |
| Kiall | other bit? | 16:05 |
| mugsie | fixing the storage layer - there is comment in the bug | 16:06 |
| Kiall | I guess I still don't see that as a bug... | 16:06 |
| Kiall | It's a relational DB - we have the info available, we just need to make use of it | 16:07 |
| mugsie | well, I do. I will file it, and we can discuss it later | 16:07 |
| vinod1 | okay so I will fix mdns's _handle_record_query the same as axfr - first get the domain_id and then query based on the domain_id and type | 16:08 |
| Kiall | vinod1: sounds good :) | 16:08 |
| mugsie | vinod1: yup | 16:08 |
| mugsie | vinod1: kiall's query above will drop it to one query with a join | 16:09 |
| vinod1 | mugsie: I will let you file the storage layer bug/non-bug :-) | 16:09 |
| mugsie | so its not quite as bad as 2 queries | 16:09 |
| Kiall | mugsie: thinking about it, the core use case for mDNS is AXFR, which has to (I think) load the domain row anyway.. | 16:10 |
| Kiall | for other queries arriving at mDNS, the smaller/simpler 2 query fix might be fine.. | 16:10 |
| mugsie | yup - if you read my comment in the bug, I said this ;) | 16:10 |
| Kiall | anyway.. whatever works ;) | 16:10 |
| vinod1 | just so that I know this for later, do I run the sql query as conn.execute similar to https://github.com/openstack/designate/blob/master/designate/storage/impl_sqlalchemy/migrate_repo/versions/038_icehouse.py#L216 | 16:12 |
| vinod1 | Kiall/mugsie: For record types other than SOA I see an issue | 16:18 |
| vinod1 | When we get a request for a recordset type - we just get the name and type | 16:18 |
| vinod1 | The name need not be the same as the domain name | 16:18 |
| vinod1 | So from the recordset name we need to figure out the domain name for this to work correctly | 16:19 |
| vinod1 | or we need to get all the recordsets for that name and type and then see which ones are deleted based on the domain_id | 16:19 |
| vinod1 | so we need to run the query that you mentioned earlier | 16:20 |
| Kiall | vinod1: humm.. crap. | 16:20 |
| Kiall | re the Q on running the query, no.. conn.execute() should be avoided! You can build up the same query using SQLA Core | 16:21 |
| Kiall | But.. implementing it is probably going to be painful - looking at the sqla driver now to see what we can do... | 16:22 |
| Kiall | https://github.com/openstack/designate/blob/master/designate/storage/impl_sqlalchemy/__init__.py#L164 | 16:24 |
| Kiall | if we update that method to take in a query as well as the table, and have all the other methods pass in `select([table])` as the query, we have an opportunity to customize the query a little... So, _find_recordsets could then pass `select([tables.recordsets]).join(tables.domains).where(tables.domains.id == "0")` | 16:27 |
| Kiall | So, _find_recordsets could then pass `select([tables.recordsets]).join(tables.domains).where(tables.domains.deleted == "0")`* | 16:27 |
| *** mwagner_lap has quit IRC | 16:29 | |
| Kiall | the only thing I dislike about that is... all our calls to find_recordsets etc will end up doing the join | 16:30 |
| vinod1 | We can have a new function for this - find_active_recordsets ? | 16:30 |
| Kiall | Yea, we could | 16:33 |
| Kiall | Well.. Is it worth it? Is the JOIN going to cause much DB pain? It's a single primary key lookup on a smaller table, and we're not actually selecting the data.. | 16:35 |
| Kiall | vinod1 / mugsie ... thoughts? | 16:35 |
| mugsie | Kiall: its not just a single PK look up though :| | 16:36 |
| mugsie | its a filter on an unindexed field (domains.deleted) | 16:36 |
| mugsie | on what could be a massive table | 16:37 |
| Kiall | deleted is indexed | 16:37 |
| mugsie | is it? | 16:37 |
| mugsie | ok | 16:37 |
| Kiall | (a unique index is on it) | 16:37 |
| mugsie | what data type is it? | 16:37 |
| Kiall | varchar(32) ish | 16:37 |
| mugsie | hrum | 16:37 |
| vinod1 | we would have at least a lot of SOA record requests from the servers based on the refresh rate of the domains. | 16:38 |
| Kiall | I'm not sure it matters.. So.. The JOIN acts on the PK to get the row, then looks at the data | 16:38 |
| Kiall | I think :/ | 16:38 |
| mugsie | yeah - but the join will load all rows | 16:38 |
| Kiall | There's only 1 matching row | 16:38 |
| mugsie | it will have to partly load them to filter | 16:39 |
| Kiall | JOIN `domains` ON `recordsets`.`domain_id` = `domains`.`id` <-- That'll match 1 | 16:39 |
| mugsie | yeah, but all domains (even old deleted ones) will have RRsets | 16:39 |
| Kiall | vinod1: true, which suggests that the majority of queries are going to involve the join anyway, so, it's probably not worth adding a separate code path for it... | 16:40 |
| Kiall | mugsie: ah.. Yes.. Humm. | 16:40 |
| mugsie | temporary fix - join | 16:40 |
| mugsie | so for j-3 we do ^ | 16:40 |
| mugsie | and then we revisit a soft delete / other options in K | 16:40 |
| mugsie | mDNS is an experimental feature in Juno anyway | 16:41 |
| Kiall | vinod1: ekarlso- just got designate stuff for rally merged... Maybe we just try it without the extra codepath, then benchmark before after to see the actual difference ;) | 16:41 |
| mugsie | Kiall: does the designate stuff do lookups on mDNS? | 16:41 |
| mugsie | i thought it just added the API calls | 16:41 |
| Kiall | mugsie: the JOIN would be in the storage layer, so even API calls etc would get it | 16:41 |
| mugsie | (currently) | 16:41 |
| Kiall | "does the designate stuff do lookups on mDNS?" Not sure what you mean? | 16:42 |
| mugsie | we cant test the perfomance difference with out doing a dig style query @ mdns | 16:42 |
| mugsie | curretnyl the rally code just hits the API | 16:43 |
| mugsie | i think a separate code path for j-3 is OK | 16:43 |
| Kiall | Right - but the find_recordsets method is used by both the API and mDNS ;) | 16:43 |
| mugsie | s/j-3/juno/ | 16:43 |
| vinod1 | Kiall - just to clarify _handle_record_query uses find_recordset | 16:45 |
| *** eankutse has joined #openstack-dns | 16:45 | |
| Kiall | Yep, it does :) | 16:46 |
| vinod1 | So now I will change find_recordset, rather than add a new api - any objections? | 16:47 |
| mugsie | vinod1: no - for juno create a new method | 16:48 |
| Kiall | vinod1: I'm happy with that - once you have it I'll run a before/after benchmark.. | 16:48 |
| Kiall | lol | 16:48 |
| *** betsy has joined #openstack-dns | 16:48 | |
| Kiall | can't agree on anything today ;) | 16:48 |
| *** paul_glass1 has joined #openstack-dns | 16:49 | |
| vinod1 | How about adding an optional parameter - query to find_recordset? | 16:49 |
| vinod1 | def find_recordset(self, context, criterion, query) | 16:49 |
| *** paul_glass has quit IRC | 16:51 | |
| Kiall | Humm, I think we should keep it hidden in the storage layer, so `select([tables.recordsets]).join(tables.domains).where(tables.domains.id == "0")` would be in https://github.com/openstack/designate/blob/master/designate/storage/impl_sqlalchemy/__init__.py#L543 | 16:51 |
| Kiall | and then passed to _find | 16:51 |
| vinod1 | I am fine with it - mugsie? | 16:55 |
| vinod1 | whether we do it this way or add a new api - the hit on mdns would be the same | 16:55 |
| mugsie | i personally think forcing the join on everything is bad idea, that will be very difficult to benchmark | 16:55 |
| mugsie | yeah - the hit to mdns will be | 16:56 |
| mugsie | but the hit to api etc wont be | 16:56 |
| mugsie | we are talking about potentialy harming our main interface, for the sake of an experimental interface that no one should be deploying in production in junp | 16:57 |
| mugsie | juno* | 16:57 |
| mugsie | which i think is crazy to even consider | 16:57 |
| Kiall | I don't like the idea of adding single purpose methods to the storage API, I'd personally prefer to keep that clean... | 16:57 |
| mugsie | fine | 16:57 |
| mugsie | but causing potential for havok in the API is mental to consider in the RC phase of a release | 16:58 |
| Kiall | I'm not sure I see the potential for havok, either it works or it doesn't, and I believe we can benchmark the before/after with a high enough confidence to make the call on if we merge or not. | 16:59 |
| mugsie | and, I dont think we can properly. you are taking about 1/2 loading the entire domains table every time some one does a get request on a single record. | 17:00 |
| mugsie | think about that. | 17:00 |
| Kiall | If that's really the result, 1/2 the domain's table get loaded on each go.. That will be blindingly obvious in even a trivial before/after benchmark.... | 17:01 |
| mugsie | i think this warrents a full discussion - before we implemnent anything from it. | 17:02 |
| mugsie | I am really not comfortable doing it this way. | 17:02 |
| Kiall | I get you don't agree, that's fine.. Calling for a vote because you don't agree with a suggestion to *try it and see if it OK* is pushing it IMO. | 17:03 |
| mugsie | well, I think that we should discuss it with the wider team group before we do this - thats not pushing it | 17:03 |
| Kiall | Anyway - I've got a demo to give! We can continue this later if needed... | 17:03 |
| mugsie | that is following the proces | 17:03 |
| vinod1 | How about applying the join only when the criterion does not include a domain_id? Most of the api calls should have the domain_id | 17:04 |
| mugsie | vinod1: i think they all should | 17:04 |
| *** openstackgerrit has quit IRC | 17:04 | |
| mugsie | yeah - from the looks of it they do, bar the floating_ip stuff | 17:06 |
| mugsie | most of the pass a full recordset object back, which has a domain_id in it | 17:08 |
| mugsie | so, that might work | 17:08 |
| mugsie | we would want to test the floating ip stuff thougharly as well | 17:08 |
| vinod1 | ok - let me work on the fix | 17:09 |
| *** HenryG is now known as HenryThe8th | 17:12 | |
| *** rossk has joined #openstack-dns | 17:12 | |
| *** diga has joined #openstack-dns | 17:26 | |
| *** mwagner_lap has joined #openstack-dns | 17:47 | |
| *** jmcbride has quit IRC | 18:03 | |
| *** EricGonc_ has quit IRC | 18:03 | |
| *** paul_glass1 has quit IRC | 18:06 | |
| *** betsy has quit IRC | 18:06 | |
| *** timsim has quit IRC | 18:07 | |
| *** vinod1 has quit IRC | 18:07 | |
| *** eankutse has quit IRC | 18:09 | |
| *** openstackgerrit has joined #openstack-dns | 18:10 | |
| *** ChanServ sets mode: +v openstackgerrit | 18:10 | |
| *** openstackgerrit has quit IRC | 19:01 | |
| *** openstackgerrit has joined #openstack-dns | 19:02 | |
| *** ChanServ sets mode: +v openstackgerrit | 19:02 | |
| *** mwagner_lap has quit IRC | 19:20 | |
| *** darshan104 has joined #openstack-dns | 19:55 | |
| *** arborism has joined #openstack-dns | 20:26 | |
| *** EricGonczer_ has joined #openstack-dns | 20:44 | |
| *** EricGonczer_ has quit IRC | 20:56 | |
| *** EricGonczer_ has joined #openstack-dns | 20:57 | |
| *** mwagner_lap has joined #openstack-dns | 21:21 | |
| *** nkinder has quit IRC | 21:26 | |
| *** openstackgerrit has quit IRC | 21:31 | |
| *** vinod1 has joined #openstack-dns | 21:32 | |
| *** openstackgerrit has joined #openstack-dns | 21:32 | |
| *** ChanServ sets mode: +v openstackgerrit | 21:32 | |
| *** vinod1 has quit IRC | 21:36 | |
| *** vinod1 has joined #openstack-dns | 21:39 | |
| *** EricGonczer_ has quit IRC | 22:08 | |
| *** darshan104 has quit IRC | 22:35 | |
| *** darshan104 has joined #openstack-dns | 22:35 | |
| *** EricGonczer_ has joined #openstack-dns | 22:45 | |
| *** arborism is now known as amcrn | 22:46 | |
| *** EricGonczer_ has quit IRC | 23:01 | |
| *** darshan104 has quit IRC | 23:05 | |
| openstackgerrit | Vinod Mangalpally proposed a change to openstack/designate: _find_recordsets returns active recordsets only https://review.openstack.org/119502 | 23:11 |
| *** richm has quit IRC | 23:17 | |
| *** vinod1 has quit IRC | 23:27 | |
| *** vinod1 has joined #openstack-dns | 23:30 | |
| *** vinod1 has quit IRC | 23:36 | |
| *** vinod1 has joined #openstack-dns | 23:48 | |
| *** vinod1 has quit IRC | 23:56 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!