Skip to content

[ML] fixing inference reference counting tests#59453

Merged
benwtrent merged 2 commits intoelastic:masterfrom
benwtrent:feature/ml-inference-ref-count-test-fix
Jul 13, 2020
Merged

[ML] fixing inference reference counting tests#59453
benwtrent merged 2 commits intoelastic:masterfrom
benwtrent:feature/ml-inference-ref-count-test-fix

Conversation

@benwtrent
Copy link
Copy Markdown
Member

@benwtrent benwtrent commented Jul 13, 2020

The reference count is eventually consistent as actions are taken to account for needlessly removing the model's bytes from the circuit breaker.

See:

loadedModel.acquire();
localModelCache.put(modelId, new ModelAndConsumer(loadedModel, consumer));
shouldNotAudit.remove(modelId);
} // synchronized (loadingListeners)
for (ActionListener<LocalModel> listener = listeners.poll(); listener != null; listener = listeners.poll()) {
loadedModel.acquire();
listener.onResponse(loadedModel);
}
// account for the acquire in the synchronized block above
loadedModel.release();

closes #59445

@benwtrent benwtrent added >test Issues or PRs that are addressing/adding tests :ml Machine learning v8.0.0 v7.9.0 labels Jul 13, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/ml-core (:ml)

@benwtrent benwtrent requested a review from davidkyle July 13, 2020 18:45
@davidkyle
Copy link
Copy Markdown
Member

I just pushed a mute in dea030f that is probably your conflict sorry

Copy link
Copy Markdown
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks

@benwtrent benwtrent merged commit ac8715a into elastic:master Jul 13, 2020
@benwtrent benwtrent deleted the feature/ml-inference-ref-count-test-fix branch July 13, 2020 19:54
davidkyle pushed a commit to davidkyle/elasticsearch that referenced this pull request Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:ml Machine learning >test Issues or PRs that are addressing/adding tests v7.9.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] org.elasticsearch.xpack.ml.inference.loadingservice.ModelLoadingServiceTests.testReferenceCountingForPipeline

4 participants