-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
ITKafkaIndexingServiceTest fixes #3872
Conversation
…ying send fixed number of events more fixes
@b-slim regarding your interrupted exception comment I think it is not necessary for integration test but let me know if you strongly think otherwise |
@b-slim nevermind changed the code to handle InterruptedException |
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.
looks good, had minor comments.
|
||
// these are used to compute the expected aggregations | ||
int added = 0; | ||
int num_events = 0; | ||
|
||
// send data to kafka | ||
while (dt.compareTo(dtStop) < 0) { // as long as we're within the time span | ||
while (num_events < NUM_EVENTS_TO_SEND) { // as long as we're within the time span |
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.
The comment is no longer accurate since the event count is fixed now.
private static final String INDEXER_FILE = "/indexer/kafka_supervisor_spec.json"; | ||
private static final String QUERIES_FILE = "/indexer/kafka_index_queries.json"; | ||
private static final String DATASOURCE = "kafka_indexing_service_test"; | ||
private static final String TOPIC_NAME = "kafka_indexing_service_topic"; | ||
private static final int MINUTES_TO_SEND = 4; | ||
private static final int NUM_EVENTS_TO_SEND = 60; | ||
private static final long WAIT_TIME_MILIIS = 2 * 60 * 1000L; |
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.
Misspelled, should be WAIT_TIME_MILLIS
dtLast = dt; | ||
dt = new DateTime(zone); | ||
} | ||
|
||
producer.close(); | ||
|
||
LOG.info("Waiting for [%s] millis for Kafka indexing tasks to consume events", WAIT_TIME_MILIIS); |
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.
Would be nicer to wait up to WAIT_TIME_MILLIS
so if the tasks load faster than that, the test can exit earlier. But, this is fine for now, as ITs are not expected to be the fastest things in the world.
@gianm took care of your comments |
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.
thx 👍
* remove wait between sends, wait for ingestion to complete before querying send fixed number of events more fixes * handle interrupted exception * remove while * review comments
loadstatus
does not consider realtime segments