-
Notifications
You must be signed in to change notification settings - Fork 179
Isolation #306
base: master
Are you sure you want to change the base?
Isolation #306
Conversation
Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
Signed-off-by: Goutham Veeramachaneni <[email protected]>
@fabxc @brian-brazil DONE! This is ready for review. Having a single Head and not multiple made it possible to have 0 public interface changes! I'll run this in prombench overnight for any surprises :) |
@@ -130,10 +130,17 @@ type BlockReader interface { | |||
Tombstones() (TombstoneReader, error) | |||
} | |||
|
|||
// Appendable defines an entity to which data can be appended. | |||
type Appendable interface { |
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.
Isn't there an incompatibility issue with removal of public interface?
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.
Yes, but we don't use this interface in tsdb itself. I am not sure we want to provide any guarantees about stability yet.
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.
Sure, but TSDB is a library. Anyone can depends on that including Prometheus itself. But I don't know, maybe the policy for TSDB is just to always vendor and pin to some version, so this is fine.
Signed-off-by: Goutham Veeramachaneni <[email protected]>
@gouthamve if you want to rebase and fix the conflicts I would try to review it quickly. best to do it after merging the other pending PRs that are ready for merge. |
A rebase of #105
fixes #260
fixes prometheus/prometheus#1893
Tests are broken and cleanup pending.
This change is