-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Unit Tests for Bloom Filter Enablement #19055
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
base: main
Are you sure you want to change the base?
Conversation
❌ Gradle check result for 0b314d2: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
This PR is stalled because it has been open for 30 days with no activity. |
❌ Gradle check result for dca8fea: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for e1e84e1: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 946e372: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Mandira Ghosh <[email protected]>
…ingsFormat is called Signed-off-by: Mandira Ghosh <[email protected]>
Signed-off-by: Mandira Ghosh <[email protected]>
Signed-off-by: Mandira Ghosh <[email protected]>
946e372
to
0d04e63
Compare
❌ Gradle check result for 0d04e63: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
logger = mock(Logger.class); | ||
} | ||
|
||
public void testFuzzySetEnabled() { |
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.
we are only testing with Open search Current Version, are these feature only introduced in current version. if yes, can we add test on the behaviour of previous OS version, applicable for the below tests as well.
|
||
public void testBloomFilterSerializationDeserialization() throws IOException { | ||
int elementCount = randomIntBetween(1, 100); | ||
long maxDocs = elementCount * 10L; // Keeping this high so that it ensures some bits are not set. |
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.
why are we removing these comments?
import java.io.IOException; | ||
import java.util.Iterator; | ||
import java.util.List; | ||
import java.util.*; |
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.
nit: please avoid wildcard imports
try (IndexWriter writer = new IndexWriter(dir, iwc)) { | ||
for (int i = 0; i < 100; i++) { | ||
Document doc = new Document(); | ||
doc.add(new TextField("field1", "value" + i, Field.Store.YES)); |
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.
does fuzzy filter only work for TextField, can we also add cases with other field types?
} | ||
long lookupTime = System.nanoTime() - startTime; | ||
|
||
logger.info("Bloom filter performance - Insertion time: {} ns/item, Lookup time: {} ns/lookup", insertionTime / expectedItems, lookupTime / lookups); |
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.
we are just logging the performance log? since the expectedItems are defined, can we create an approx assertion on the lookups times? like time are in the range of 20-50% or less than 50% of baseline, can this lead to flaky test?
Added unit tests for FuzzySet to check if FuzzySet is enabled and when it is enabled, it is calling FuzzyFilterPostingsFormat. If it is, then bloom filter is working when enabled. Also added few more unit test to test new codec.