search

palantir / atlasdb

Transactional Distributed Database Layer
https://palantir.github.io/atlasdb/
Apache License 2.0
54 stars 9 forks source link

Namespace AtlasDB endpoints #1714

Open ashrayjain opened 7 years ago

ashrayjain commented 7 years ago

This will help consumers reliably reference all AtlasDB endpoints which is non trivial in the current world. An example of when this has caused problems is the recent addition of the the new /persistent-lock endpoints that went undetected in the internal navigational product and are consequently not whitelisted in terms of requiring auth.

@jboreiko

jboreiko commented 7 years ago

@joelea for some reason I remember us not doing this when we talked about it before. Do you happen to remember why?

joelea commented 7 years ago

I think the issue we had with this before was when a product had a URL scheme of <bucket>/<data> so you couldn't create a bucket called lock and it wouldn't have really helped in that case. Given this, it feels like it would probably be a really frustrating change to make for not much benefit.

It's also a bit of a bandaid over the more structural issue that timelock is designed to solve, so we might have just dumped it as not worth the short term gains.

rhero commented 7 years ago

Given that Timelock doesn't have a dependency on the KVS, we'll have to keep the /persistent-lock endpoint with the AtlasDB client. We could probably do this while keeping the old endpoint for some time (routing both to the same call), then deprecate and remove the old one.

@ashrayjain do you want to put up a PR for this? I assume you'll want this before the rollout of your product, so we can try and get to it soon.

joelea commented 7 years ago

Obviously I'm not going to be the one dealing with the consequences of the decision any more (and I'm pretty low on context), but if persistent-lock is going to be the last endpoint that atlas forces it's consumers to host amongst their own endpoints I'd bias towards having Timelock take a KVS dependency over leaving the endpoint in place.

If there's a future where persistent-lock exists as an endpoint without a KVS dependency, then either situation is temporary. However, if it ends up sticking around (for technical or prioritization reasons) then I'd want my APIs to be sane and my deps to be a bit funky than the other way round.

ashrayjain commented 7 years ago

As suggested by @SerialVelocity in our discussions, we can actually remove the KVS dependency for the persistent-locks and so it feels reasonable to move it to Timelock.

tpetracca commented 7 years ago

I love the idea of ripping persisted-lock stuff out of the KVS and into TimeLock. We should sit down and talk that through.

Are there any remaining atlas endpoints that timelock doesn't now own other than persisted/backup lock stuff?

ashrayjain commented 7 years ago

@tpetracca there will probably need to be stuff like clean transactions range, etc. eventually. Created #1767 for tracking moving persistent lock to timelock separately.