-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[TST] WIP - NOCOMMIT - Testing #4522
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
Please tag your PR title with one of: [ENH | BUG | DOC | TST | BLD | PERF | TYP | CLN | CHORE]. See https://docs.trychroma.com/contributing#contributing-code-and-ideas |
Fix Version-based Wait Issues in Distributed Tests This PR fixes bugs in the wait_for_version_increase mechanism in distributed tests. It resolves a race condition by capturing the initial version before performing operations, then waiting for a version increase compared to that specific version, rather than potentially using an already-increased version as the comparison baseline. Key Changes: Affected Areas: This summary was automatically generated by @propel-code-bot |
Please tag your PR title with one of: [ENH | BUG | DOC | TST | BLD | PERF | TYP | CLN | CHORE]. See https://docs.trychroma.com/contributing#contributing-code-and-ideas |
@@ -115,7 +115,7 @@ impl Log { | |||
} | |||
} | |||
|
|||
#[tracing::instrument(skip(self), err(Display))] | |||
#[tracing::instrument(skip(self), ret, err(Display))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[BestPractice]
The tracing instrumentation for get_collections_with_new_data
is now improved with the addition of ret
to include return values in traces, which is helpful for debugging and monitoring. This will make it easier to trace and debug issues with collection data retrieval.
@@ -371,6 +371,7 @@ impl GrpcLog { | |||
._get_collections_with_new_data(false, min_compaction_size) | |||
.await?; | |||
if self.config.use_alt_host_for_everything | |||
|| !self.config.use_alt_for_tenants.is_empty() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[BestPractice]
Added support for alternative tenants routing with use_alt_for_tenants
in the condition check. This is an important addition for the distributed setup to properly handle tenant routing to alternative hosts.
It would add and delete items in such a way that it's theoretically possible to have a version increase before checking the current version. Waiting for "current", then, actually begs the question and waits for itself.
Description of changes
I'm adding debug statements to catch a bug.
Test plan
CI is the test plan.
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
N/A