Code review comment for lp:~lamont/maas/bug-1372544

Revision history for this message
Gavin Panella (allenap) wrote :

I got a bit carried away reviewing this. I built on what you've done and came up with quite a large bunch of changes. I've made the evaluations more efficient and put everything into post-commit hooks. Take a look at lp:~allenap/maas/lamont--bug-1372544 (or the merge proposal at https://code.launchpad.net/~allenap/maas/lamont--bug-1372544/+merge/294167).

In time, a better solution might be to model a work queue, like we did for DNS zones recently. That way we can better manage the work in an HA environment, and better recover from failures. For example, we really don't want to be doing lots of tag evaluation work for more than one tag update at a time otherwise we run a real risk of DOSing the MAAS installation. There's no protection against that right now.

(We could add the DOS protection to this current implementation quite easily, but recovery would still need a queue, or some other additional support in the data model.)

Anyway, look at what I've done. If you like it, merge it into this, write some tests for `populate_tag_for_multiple_nodes` and it should be good to land.

« Back to merge proposal