test: Add delay to L0_lifecycle test_load_new_model_version after each model file update #7735
Conversation
kthui
force-pushed
the
jacky-lifecycle-model-version
branch
from
95b1c55
to
1af39e8
Compare
There was a problem hiding this comment.
Just to check my understanding, is the flakiness coming from checks for model versions expected to be "not ready"?
I.e. The calls to "load" in some test cases are actually triggering a "reload" that may "unload" some instances (removed versions from from the diff of new config), and the server doesn't wait for each unload to completely finish before returning to the client, so the checks for "not ready" models may fail if it hasn't fully unloaded on server side yet?
I would expect "loads" to block on server side until actually complete and model considered ready before returning to client. Whereas I believe that "unloads" may not block until considered "not ready", so I want to double check if the flakiness is coming from one of these "unload" cases via a version-policy-change-only "reload".
|
I looked deeper into the failed server and client log, and found the last load operation was successful according to the client log, but according to the server log the load operation did not commence. Subsequently, the check for model version 1 is loaded failed. If I remove the model config update from the test, I was able to reproduce the same client and server log locally. Base on the finding, my hypothesis for the failure is the model config update has been written to disk, according to the Python script, which at that point the server should see the updated file, but for some reason the server was seeing the old version by the time the load operation arrived. I will update the test accordingly. |
|
The test script on the container is updated to the latest and re-ran, see job 118303410 |
There was a problem hiding this comment.
Moving to thread.
Base on the finding, my hypothesis for the failure is the model config update has been written to disk, according to the Python script, which at that point the server should see the updated file, but for some reason the server was seeing the old version by the time the load operation arrived. I will update the test accordingly.
Is there a bug in checking file modification time or something?
There was a problem hiding this comment.
I think this is unique to the CI, because I ran the test from the failing container locally 10 times and they all passed. I ran the test ~560 times last night on the CI without any time.sleep() and they all passed, see job 118186442.
Edit: The ~560 times CI run has --log-verbose=2, which could change some timing.
There was a problem hiding this comment.
I think another possibility is the timestamp is the same before and after the file update, but this breaks our assumption that a ns precision timestamp is sufficient at determining the order of events.
kthui
changed the title
kthui
changed the title
What does the PR do?
Add a small delay to L0_lifecycle test_load_new_model_version after each model file update, to prevent flaky results due to the update not being picked up by the server during model load request.
Checklist
<commit_type>: <Title>Commit Type:
Check the conventional commit type
box here and add the label to the github PR.
Related PRs:
N/A
Where should the reviewer start?
Start by looking into the L0_lifecycle failure of pipeline 19628457
Test plan:
L0_lifecycle pass after the patch.
Caveats:
N/A
Background
See #7730 and job 117883843
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
N/A