Bring back old functionality of Checkpoint()

[codesome]

As pointed by @fabxc here #396 (comment), Checkpoint functionality should not be changed as it is exposed.

In order to preserve the new metrics added in #396, there are 2 new arguments for Checkpoint(...) (can be nil).

/cc @krasi-georgiev

Signed-off-by: Ganesh Vernekar [email protected]

[brian-brazil]

TSDB is an internal library, we don't make any promises about API stability on those.

[fabxc]

TSDB is an internal library, we don't make any promises about API stability on those.

We never formalized rules but we know that our libraries have external users and that breaking exported APIs causes friction. So I think it's fair to at least require a strong-ish reason to break.

What bothered me in this case was that semantics where changed significantly while code will keep compiling as-is – so undetectable with fatal-ish consequences.
Restoring the old behavior in this PR of course caused another breaking change and the signature is quite a bit more convoluted.

I'm on the fence about it – happy to hear other opinions.

[krasi-georgiev]

What bothered me in this case was that semantics where changed significantly while code will keep compiling as-is – so undetectable with fatal-ish consequences.

my bad, sorry!

in regards to changing exported APIs for packages , what about making a new release when we need to change an api describing the change so that people vendoring the package would at least read the release notes?

[simonpasquier]

When I reviewed #396, it crossed my mind that changing the internals of Checkpoint() was modifying the API contract but I wasn't sure what is the level of stability promised by tsbd. Somehow this relates with #381: if we're better at producing tsdb releases, it would both help Prometheus and external consumers of the library.

[fabxc]

my bad, sorry!

No need to apologize – it's a valid refactoring in general, given something had to be broken either way for the new metrics. Maybe it's alright since the API is rather fresh and not super widely used yet.

Proper library releases with tags and notes sound like a good idea. I guess we are not doing that fur libraries in prometheus/prometheus. But arguably the consequences there are nowhere as bad as for TSDB.

[krasi-georgiev]

ok , lets leave the current implementation as is and will try with the releases when we have exposed API changes.

With the first one will also make a short doc in the wiki(so it is easy to update) for the release process and once it works well will add it somewhere in the repo as well.