HTCondor Project List Archives



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Condor-devel] RFC: A corner case in classad scoping for select/subscript operator



While I was constructing an example configuration for min/max resources,
I created a fairly confusing situation where I indirectly referenced an
attribute (in fact, two: AccountingGroup and ConcurrencyLimits) which
appear in both the machine ad and the job ad.

There's a discussion of the specific configuration here. In the
abstract, I had something like this:

machine ad:
[ ...
requirements = e1(other.AG)
... ]

where e1 is an expression including the attribute name "AG".  Now, "AG"
is defined on the *job* ad, like so:

job ad:
[...
AG = e2(AccountingGroup)  # an expression involving the attr name
"AccountingGroup"
...]


When it came time to test a match by evaluating the requirements, it
evaluated:

eval(e1(e2(AccountingGroup)), machine_ad)

that is, it deferred evaluation of e2(AccountingGroup), and then used
the value of AccountingGroup scoped by the machine ad, where what I
really wanted (and expected) was AccountingGroup scoped to the job ad.

What I did to fix it was make scope more explicit(*) on the job ad side:

job ad:
[...
AG = e2(my.AccountingGroup)  # scopes AccountingGroup to job ad
...]

I'm curious about whether this deferred evaluation, under the machine ad
scope, is a correct interpretation of the classad language spec (Tim's
assessment is "yes it is", and he is quite possibly right).  The
alternative interpretation would be that 'other.AG' evaluates as:
eval(e2(AccountingGroup), job_ad) - that is, the RHS is evaluated in the
scope of the base "other".

I consulted the spec:
http://research.cs.wisc.edu/condor/classad/refman/node4.html

and strangely enough, the spec itself literally is missing that
information on scoping: the nut of the definition is here:

"If Es' is a String with value s and Eb' is a Record expression, then
(E', C') = eval(lookup(s,(Eb',Cb')))"

But the eval() function requires a scope as its second argument, and
that 2nd arg is missing in the definition, which happens to be exactly
the thing that clarifies what the scope should be according to the spec!

I'm interested in any community insights into what that 2nd scoping arg
was intended to be.


(*) Actually, if the expected behavior is:
        eval(e1(e2(my.AccountingGroup)), machine_ad)

then arguably a user shouldn't expect that using "my." would fix it
either, since one might expect "my." to then refer to machine_ad.  

In practice "my." is pre-substituted and that is why it works, but it
seems to conflict with the eval logic where evaluation of other.AG -->
eval(e1(e2(AccountingGroup)), machine_ad).  Although it allowed me to
get what I needed, it seems that if the one didn't work, this arguably
shouldn't work either.

That is, presubstituting "my.", "other.", and friends to ".left" and
".right" is a condor thing, but not exactly a classad language spec
thing.  Apparently done to make eval go faster.  The construction that
is described in the header doc for matchClassAd.h (but not actually
built as described) is more aligned with the spec, although it would be
slower:

    [
       symmetricMatch   = leftMatchesRight && rightMatchesLeft;
       leftMatchesRight = RIGHT.requirements;
       rightMatchesLeft = LEFT.requirements;
       leftRankValue    = LEFT.rank;
       rightRankValue   = RIGHT.rank;
       RIGHT            = rCtx.ad;
       LEFT             = lCtx.ad;
       lCtx             =
           [
               other    = .RIGHT;
               target   = .RIGHT;   // for condor backwards compatibility
               my       = .LEFT;    // for condor backwards compatibility
               ad       = 
                  [
                      // the ``left'' match candidate goes here
                  ]
    	   ];
       rCtx             =
           [
               other    = .LEFT;
               target   = .LEFT;    // for condor backwards compatibility
               my       = .RIGHT;   // for condor backwards compatibility
               ad       = 
                  [
                      // the ``right'' match candidate goes here
                  ]
    	   ];
    ]