[engine] Eliminate non-determinism computing cache keys

Review Request #3593 - Created March 21, 2016 and submitted

Information
Peiyu Wang
pants
3066
Reviewers
pants-reviews
kwlzn, patricklaw, stuhood

There is an existing unit test that verify reruning the same request hits cache
100%, but that's within the same python process. As it turns out there is
randomness when persisting cache to local path and reuse from restarting a
different process.

Have identified two sources for non-determinism:

  • StepRequest.step_id this could change as when requests may be executed,
    returned and scheduled in different order. Alternatives considered
    1) remove step_id from StepRequest, since we still pass step_id to and
    back from sub process, the interface would look less clean.
    2) overwrite StepRequest__reduce__ provide fields that are to be pickled,
    essentially whitelisting the fields, that's fragile if we ever add/remove
    fields

  • StepRequest.dependencies is an unsorted map. So sorting is provided in this
    review that sorts in two steps:
    1) group nodes by their types, since comparison for nodes of different types
    is not defined
    2) add Key.__lt__, currently undefined which defaults to identity based.

Implemented StepRequest.compute_key to handle the above two.

Manual testing on list 3rdparty/:: now gets 100% hit rate, was <20%, this
results about 3x improvement speed up.

Also fixed a bug that is_cacheable is missing @property, this results every
node is_cacheable will return True, therefore FilesystemNode is cached!
AddressMapperTest.test_no_address_no_family is supposed to catch that, but
when we invalidate the graph we don't reset step_id therefore in the second run
requests get higher step_ids, i.e, different fingerprints. Now with the step_id
change the bug properly shows up.

https://travis-ci.org/peiyuwang/pants/builds/117578595 passed
http://jenkins.pantsbuild.org/job/pants_ci.unit_tests/136/ passed

manually tested list 3rdparty/::

Issues

  • 0
  • 3
  • 0
  • 3
Description From Last Updated
Stu Hood
Peiyu Wang
Peiyu Wang
Peiyu Wang
Stu Hood
Peiyu Wang
Review request changed

Status: Closed (submitted)

Change Summary:

Submitted as 26386a87caa36cc800f3604b113f5fae85947396

Loading...