Skip to content

Handle mispredictions cleanly. #12

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

Closed
wants to merge 1 commit into from

Conversation

jcjones
Copy link
Collaborator

@jcjones jcjones commented Jun 8, 2021

In the real world, we might mess up when naming a partition. This should be
rare if partitionmanager is running often, since it'll rename partitions
to match reality, but when it's running only rarely, things get out of date.

This change avoids attempting to calculate rates-of-change using partitions
that don't make sense - e.g., today is July 1, and our active partition
says it starts in a week. That is plainly wrong, but we can still use our
current rate-of-change.

In the real world, we might mess up when naming a partition. This should be
rare if partitionmanager is running often, since it'll rename partitions
to match reality, but when it's running only rarely, things get out of date.

This change avoids attempting to calculate rates-of-change using partitions
that don't make sense - e.g., today is July 1, and our active partition
says it starts in a week. That is plainly wrong, but we can still use our
current rate-of-change.
@jcjones jcjones requested a review from aarongable June 8, 2021 22:34
@jcjones
Copy link
Collaborator Author

jcjones commented Sep 20, 2021

This was rolled into PR #4

@jcjones jcjones closed this Sep 20, 2021
@jcjones
Copy link
Collaborator Author

jcjones commented Sep 20, 2021

Turns out this was only partially rolled into PR #4, and there was another edge case uncovered.

@jcjones jcjones deleted the pr-branch-partition-time-assertions branch September 20, 2021 23:52
jcjones added a commit that referenced this pull request Sep 20, 2021
In the real world, we might mess up when naming a partition. This should be
rare if partitionmanager is running often, since it'll rename partitions
to match reality, but when it's running only rarely, things get out of date.

This change avoids attempting to calculate rates-of-change using partitions
that don't make sense - e.g., today is July 1, and our active partition
says it starts in a week. That is plainly wrong, but we can still use our
current rate-of-change.

This expands on PR #12 by changing what the start-datetime is for new
partitions after we mispredicted - without this change, if we had partitions
through to December, but it's only August and we need more, the new partitions
would be named for January instead of reflecting reality that they need to
be named for Right Now.

This also catches a bug where we could get timestamp name collisions. This is
a lot less of an issue when I implement Tim's suggestion in #19, but for now
this just increases dates by a day to avoid a collision, and that works well.
jcjones added a commit that referenced this pull request Sep 21, 2021
In the real world, we might mess up when naming a partition. This should be
rare if partitionmanager is running often, since it'll rename partitions
to match reality, but when it's running only rarely, things get out of date.

This change avoids attempting to calculate rates-of-change using partitions
that don't make sense - e.g., today is July 1, and our active partition
says it starts in a week. That is plainly wrong, but we can still use our
current rate-of-change.

This expands on PR #12 by changing what the start-datetime is for new
partitions after we mispredicted - without this change, if we had partitions
through to December, but it's only August and we need more, the new partitions
would be named for January instead of reflecting reality that they need to
be named for Right Now.

This also catches a bug where we could get timestamp name collisions. This is
a lot less of an issue when I implement Tim's suggestion in #19, but for now
this just increases dates by a day to avoid a collision, and that works well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant