> On Aug. 13, 2020, 9:22 p.m., Benjamin Mahler wrote: > > src/master/offer_constraints_filter.cpp > > Lines 64 (patched) > > <https://reviews.apache.org/r/72741/diff/1/?file=2237295#file2237295line64> > > > > This is a little confusing to me since we already have a predicate type > > from the protobuf and this is essentially the same thing but in a more > > usable format for this code? > > > > I wonder if there's a way to accomplish what we want without the extra > > predicate type. Since the evaluator holds one of these predicates, maybe > > the evaluator can be the one to hold any extra predicate logic / > > information needed rather than having a duplicate predicate type.
I would rather keep constraint evaluation decomposed into attribute selection and predicate evaluation; the reason is that equality/regexp match will add noticeable amount of code into the predicate evaluation. > On Aug. 13, 2020, 9:22 p.m., Benjamin Mahler wrote: > > src/master/offer_constraints_filter.cpp > > Lines 134-143 (patched) > > <https://reviews.apache.org/r/72741/diff/1/?file=2237295#file2237295line134> > > > > I think we allow multiple entries for the same attribute key, on the > > agent side? Not sure if anyone uses that. > > Andrei Sekretenko wrote: > Good point. > We allow, and this is valid. I don't see any code/document disallowing > that; moreover, IIRC some doc mentions this explicitly. > > Unconditionally taking the first attribute with the specified name is > what Marathon uses for implementing their placement constraints. > Given that Marathon seems to be the first candidate to make use of > constraints-based filtering, I'm following this approach. > > Looks like I should document this explicitly and write a test for > applying constraint to an agent with multiple attributes having the same name. > If someone at some point will need another logic for the attribute > lookup, they will probably be able to do that by adding something like > `lookup_mode` to the `Selector` message and implementing it here. Documented in https://reviews.apache.org/r/72738/; a test is pending, as I cannot really test this with Exists constraint. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72741/#review221571 ----------------------------------------------------------- On Aug. 14, 2020, 4:42 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72741/ > ----------------------------------------------------------- > > (Updated Aug. 14, 2020, 4:42 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-10171 > https://issues.apache.org/jira/browse/MESOS-10171 > > > Repository: mesos > > > Description > ------- > > This patch implements an offer filtering object that supports the > Exists/NotExists offer constraints, and adds it into the allocator > interface. > > More constraints will be added to this filter in further patches. > > > Diffs > ----- > > include/mesos/allocator/allocator.hpp > c700528e14bb42f6cea37f42dd7b72eb76a1a6b9 > src/CMakeLists.txt 4e15e3d99aa2cce2403fe07e762fef2fb4a27dea > src/Makefile.am 447db323875e4cad46000977f4a61600baff8f89 > src/master/allocator/mesos/offer_constraints_filter.cpp PRE-CREATION > src/master/master.cpp 6a013e334b19bd108791d1c5fd0864c710aae8cb > > > Diff: https://reviews.apache.org/r/72741/diff/2/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
