[rabit harden] fix model recovery tests failures #510
Conversation
chenqin
changed the title
tracker/dmlc_tracker/local.py
Outdated
| @@ -8,38 +8,52 @@ | |||
| import logging | |||
| from threading import Thread | |||
| from . import tracker | |||
| import pdb | |||
tracker/dmlc_tracker/local.py
Outdated
| num_retry = env.get('DMLC_NUM_ATTEMPT', 0) | ||
|
|
||
| #overwrite default num of retry with commandline value | ||
| for parm in cmd: |
| @@ -11,6 +11,8 @@ | |||
| #include <dmlc/logging.h> | |||
| #include <string> | |||
| #include <mutex> | |||
| #include <utility> | |||
| #include <memory> | |||
There was a problem hiding this comment.
from cpplint failure https://travis-ci.org/dmlc/dmlc-core/jobs/503196406
There was a problem hiding this comment.
this refers to a succeeded job?
|
Added. |
|
@CodingCat note that the master branch is diverging from the one used by mxnet (tracked in #507). cc @hcho3 who's looking into it. |
|
@szha thanks! |
|
I still recommend you point to this branch in dmlc/rabit#81 for cross-validation, and it will prevent the error result by the different environment in your laptop and rabit |
|
merged, thanks |
| if num_retry >= 0: | ||
| # failure trail increase by 1 and restart failed worker | ||
| for arg in cmd: | ||
| if arg.startswith('rabit_num_trial'): |
There was a problem hiding this comment.
I might have missed this here, I don't think we should leak more rabit stuff in dmlc-core (hopefully we should rename RabitTracker), essentially they are two different projects
There was a problem hiding this comment.
Described on PR item 3, the semantic of rabit_num_trial is to capture the number of worker failures caused by mock. v.s DMLC_NUM_ATTEMPT is setting number of retries in total local tracker can do. The issue really some how up to date rabit_num_trial needs to feed back into rabit init in order to make rabit behavior as expected. Otherwise it will be infinite crash...
https://github.com/dmlc/rabit/blob/master/src/allreduce_mock.h#L164
One previous attempt was to reuse DMLC_NUM_ATTEMPT as replacement of rabit_num_trial. The problem is rabit_num_trial starts with 0 and DMLC_NUM_ATTEMPT needs to >= number of crashes for tests to pass. That basically broke tests in rabit.
There was a problem hiding this comment.
ok, in this case, why we need to pass a value as rabit_num_trial?
we should remove it from both of dmlc-core and rabit, right?
There was a problem hiding this comment.
how can restarted rabit mock worker know which trail it is running and decide if it will crash or not. There needs something pass via arguments...
https://github.com/dmlc/rabit/blob/master/src/allreduce_mock.h#L52
There was a problem hiding this comment.
I would suggest add another argument called
DMLC_WORKER_MAX_RETRY along with DMLC_NUM_ATTEMPT and remove rabit_num_trial from core.
There was a problem hiding this comment.
just DMLC_MAX_RETRY and consume rabit_num_trial from rabit
There was a problem hiding this comment.
Rabit tests has been failing since early 2018.
According to @tqchen this is largely due to tracker semantic issue. As I have been working on enable checkpoint at rabit could work on GCP. I need to make sure those tests can pass before introducing extra features.
Here is some of my observations to tracker/rabit interaction.
Rabit mock relay on command line to pass on which number of trail it runs on as a factor to simulate worker crash (exit(-2)). When worker actually dead and local tracker subprocess exit. worker were restarted without update rabit_num_trail setting. (this is essentially infinite loop of crash)
Total number of restart a worker in local tracker is controlled by DMLC_NUM_ATTEMPT, by default it was set to zero. So when a rabit unit test kick off like following, total number of retry is still set to zero (no retry) and local tracker throw exception.
Rabit Init has a overwrite code which use value from DMLC_NUM_ATTEMPT to overwrite value from rabit_num_trial. I am not sure why since num_trail is incremental and DMLC_NUM_ATTEMPT if set to greater than 0 will skip most of mock failures.
model_recover_10_10k: ../dmlc-core/tracker/dmlc-submit --cluster local --num-workers=2 model_recover 10000 mock=0,0,1,0 mock=1,1,1,0 rabit_num_trial=0 DMLC_NUM_ATTEMPT=10