https://pantsbuild.org/ logo
Powered by
#development
  • w

    witty-crayon-22786

    12/12/2017, 7:32 PM
    so the only question is 2: service lifecycle
  • r

    rough-minister-58256

    12/12/2017, 7:32 PM
    s/removed/replaced with teh fork lock/ you mean?
  • w

    witty-crayon-22786

    12/12/2017, 7:32 PM
    @rough-minister-58256: right, sorry. the only thing we're guarding is forking.
  • r

    rough-minister-58256

    12/12/2017, 7:33 PM
    I think the lifecycle is needed to safeguard against teardown vs an in-flight pailgun run
  • w

    witty-crayon-22786

    12/12/2017, 7:33 PM
    i'm differentiating these, because two threads could be safely interacting with the scheduler at once, no problem, because it has internal locking
  • w

    witty-crayon-22786

    12/12/2017, 7:33 PM
    so that's 3
  • r

    rough-minister-58256

    12/12/2017, 7:33 PM
    you can 
    watch -n.5 "pkill watchman"
    to simulate a constantly crashing daemon
  • w

    witty-crayon-22786

    12/12/2017, 7:33 PM
    ok.
  • w

    witty-crayon-22786

    12/12/2017, 7:33 PM
    so:
  • w

    witty-crayon-22786

    12/12/2017, 7:33 PM
    Copy code
    1) the fork
2) the service lifecycle thing
3) scheduler interactions
  • w

    witty-crayon-22786

    12/12/2017, 7:34 PM
    i removed 3 in #5178
  • w

    witty-crayon-22786

    12/12/2017, 7:34 PM
    you might have added some of it back
  • w

    witty-crayon-22786

    12/12/2017, 7:34 PM
    so i'll look at that
  • w

    witty-crayon-22786

    12/12/2017, 7:34 PM
    but then we'll have two separate locks for 1 and 2
  • w

    witty-crayon-22786

    12/12/2017, 7:34 PM
    right?
  • r

    rough-minister-58256

    12/12/2017, 7:34 PM
    I essentially merged the idea of these two locks on the existing PantsService.lock
  • r

    rough-minister-58256

    12/12/2017, 7:35 PM
    guarding teardown, invalidation and forking
  • r

    rough-minister-58256

    12/12/2017, 7:35 PM
    so yeah, teasing that out into two locks should be simple
  • r

    rough-minister-58256

    12/12/2017, 7:35 PM
    is #3 in your list above a thing you expect to be handled purely by rust?
  • w

    witty-crayon-22786

    12/12/2017, 7:36 PM
    ok. so step one would be for me to confirm that the integration test i added still fails.
  • w

    witty-crayon-22786

    12/12/2017, 7:36 PM
    @rough-minister-58256: yes. the native code is threadsafe.
  • r

    rough-minister-58256

    12/12/2017, 7:36 PM
    k sweet
  • w

    witty-crayon-22786

    12/12/2017, 7:36 PM
    which makes it clearer that the only reason we're holding a lock around it is to ensure that threads get shut down cleanly pre-fork
  • w

    witty-crayon-22786

    12/12/2017, 7:37 PM
    ... that lock, at least
  • r

    rough-minister-58256

    12/12/2017, 7:37 PM
    I don’t think the integration test will still fail after my PR, but thats probably besides the point since at this point its just lock optimization
  • w

    witty-crayon-22786

    12/12/2017, 7:37 PM
    yea.
  • w

    witty-crayon-22786

    12/12/2017, 7:37 PM
    ok. cool.
  • r

    rough-minister-58256

    12/12/2017, 7:37 PM
    👍
  • w

    witty-crayon-22786

    12/12/2017, 9:03 PM
    now rebasing https://github.com/pantsbuild/pants/pull/5192 ... did we both add something called 
    fork_lock
    ? heh.
  • w

    witty-crayon-22786

    12/12/2017, 9:03 PM
    i might just start over.
1...406407408...906Latest