-
Notifications
You must be signed in to change notification settings - Fork 330
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
Solving warn received an empty publish shard positions update
.
#4907
Solving warn received an empty publish shard positions update
.
#4907
Conversation
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.
This could use a unit test.
.collect::<Vec<_>>(); | ||
// This can happen if the shards have been deleted by the control plane, after building | ||
// the plan and before the apply terminated. See #4888s. | ||
info!(missing_shards=?missing_shards, "failed to acquire all requested shards"); |
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.
info!(missing_shards=?missing_shards, "failed to acquire all requested shards"); | |
info!(missing_shards=?missing_shards, "failed to acquire all assigned shards"); |
}) | ||
.collect::<Vec<_>>(); | ||
// This can happen if the shards have been deleted by the control plane, after building | ||
// the plan and before the apply terminated. See #4888s. |
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 plan and before the apply terminated. See #4888s. | |
// the plan and before the apply terminated. See #4888. |
@@ -599,7 +616,10 @@ impl Source for IngestSource { | |||
}; | |||
self.assigned_shards.insert(shard_id, assigned_shard); | |||
} | |||
self.truncate(truncate_up_to_positions).await; | |||
|
|||
if !truncate_up_to_positions.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.
We could nest this !truncate_up_to_positions.is_empty()
into truncate(...)
directly, and remove those checks elsewhere. That would make it much easier to test that we don't send empty truncates.
740fd95
to
e843726
Compare
This should remove all warn logging `received an empty publish shard positions update`. We also start logging an info showing highlighting the race condition that was causing this warning log. Closes #4888
I added a unit test in the ingest source, extended the unit test in the metastore suite, and added documentation on the spec of the metastore. |
e843726
to
da2501d
Compare
This should remove all warn logging
received an empty publish shard positions update
. We also start logging an info showing highlighting the race condition that was causing this warning log.Closes #4888