LGTM +1 On Tue, Sep 19, 2017 at 8:26 AM, Enrico Olivelli <eolive...@gmail.com> wrote:
> Looks good to me. > Enrico > > On mar 19 set 2017, 17:22 Jia Zhai <zhaiji...@gmail.com> wrote: > > > Any thoughts or comments on this. :) > > If not, would like to mark this BP approved. And we will prepare an > > initial PR for detailed discussion and comments. > > > > On Wed, Sep 13, 2017 at 4:49 PM, Sijie Guo <guosi...@gmail.com> wrote: > > > >> On Wed, Sep 13, 2017 at 1:38 AM, Enrico Olivelli <eolive...@gmail.com> > >> wrote: > >> > >> > > >> > > >> > 2017-09-13 10:31 GMT+02:00 Jia Zhai <zhaiji...@gmail.com>: > >> > > >> >> Thanks a lot for your time Yiming and Enrico. :) > >> >> > >> >> Regarding the security, we could do it in a separate BP, and make > >> this BP > >> >> more focus on filling up the useful endpoints. How about it? > >> >> > >> > > >> > OK, but please consider that the more utility you add to the endpoint > >> the > >> > more users will want to enable it and so security will be a blocker > >> issue > >> > > >> > >> My take on this - we all know security is important for any > communication > >> to bookies. > >> > >> However, security is a big different scope of problems to address. we > >> shouldn't put everything in one big BP. I'd suggest us focusing on the > >> problems > >> that a BP tends to address, defer other things to separate BPs. > Otherwise > >> we can't land any changes quickly. > >> > >> > >> > > >> > > >> >> > >> >> On Wed, Sep 13, 2017 at 3:30 PM, Enrico Olivelli < > eolive...@gmail.com> > >> >> wrote: > >> >> > >> >> > Hi Jia, > >> >> > I am OK with the idea of having standard HTTP API, this will help > >> >> > development of (non-Java) tools. > >> >> > > >> >> > It is not clear to me if we are going to add an http API useful for > >> >> > "managing bookies" or a new HTTP REST-like Client API to > BookKeeper. > >> >> > I am referring to the fact that in the proposal there are API calls > >> to > >> >> > create ledgers and to read data. > >> >> > I think we should separate this two aspects and maybe it is better > to > >> >> > address the 'bookie management' first, which is the work that > Yiming > >> >> > started. > >> >> > > >> >> [jia] It is targeting for the admin portal. the existence of the > ledger > >> >> api > >> >> is just to simplify debugging or operations. we can eliminate the > >> ‘create’ > >> >> endpoint. > >> >> > >> > > >> > OK so are you already thinking about specific tools to manage bookies > >> > without the bookie shell > >> > > >> > >> We would like to integrate this for schedulers (e.g. k8s). so we can > >> deploy > >> and operate bookkeeper easily > >> in those environments. > >> > >> > >> > > >> > > >> >> > >> >> > >> >> > > >> >> > Another point very important to me is that if we are going to > >> introduce > >> >> > management operations via http we have to take into account > >> security, at > >> >> > least TLS and some kind of authentication. > >> >> > About TLS it is very simple to achieve this, every HTTP server > >> supports > >> >> TLS > >> >> > (https) > >> >> > About authentication the problem is not so simple, Kerberos on > HTTPs > >> is > >> >> > very complex, but we need to introduce some auth mechanism. > >> >> > IMHO We can require the 'http server implementation' to implement > >> >> security > >> >> > but out of the box we have to supply basic support at least for one > >> >> > provider bundled in the distribution package. > >> >> > > >> >> [jia] The BP focuses on filling up the useful endpoints. The security > >> will > >> >> be a separate BP. > >> >> > >> >> > >> >> > > >> >> > Some API can return very large result sets like 'list ledgers', > >> actually > >> >> > the HTTP Server subsystem exchanges strings in memory, we will need > >> to > >> >> > introduce some more smart way because it would be easy to bring > down > >> the > >> >> > bookie just by calling that API multiple times concurrencly (it is > >> just > >> >> an > >> >> > example) > >> >> > > >> >> [jia] We could add pagination into all the `list` api. > >> >> > >> > > >> > Yes but it will be really tricky to implement, but please do it > >> > > >> > > >> >> > >> >> > > >> >> > IMHO The 'create ledger' API is not useful, due to the design of > BK > >> you > >> >> > have to create a ledger and then write immediately to it, I think > >> that > >> >> such > >> >> > an API should allow the client to stream the contents of the ledger > >> in > >> >> the > >> >> > HTTP body at least. But I think that a more stateful http API needs > >> to > >> >> be > >> >> > designed to implement a pure Http client > >> >> > > >> >> [jia] Thanks, we are not planning to implement an http client. we > will > >> >> remove ‘create’ here. > >> >> > >> > > >> > OK, thanks > >> > > >> >> > >> >> > >> >> > > >> >> > Maybe we should add a more narrowed motivation and add the only > >> useful > >> >> APIs > >> >> > to address those issues. > >> >> > For instance in my company we would like to start creating a > >> BookKeeper > >> >> Web > >> >> > UI, so we need some way to talk to bookies and exchange data, but > in > >> >> this > >> >> > case I am interested in asking to bookies their status and the > >> effective > >> >> > contents of the bookie > >> >> > > >> >> [jia] The BP proposes adding a standard naming convention for adding > >> admin > >> >> endpoints. Feel free to propose the endpoints you would like to > appear > >> in > >> >> the http admin portal. > >> >> > >> > > >> > The ones you wrote are enough complete for me, I would like to have > >> > read-only operation in order to have a global view on the status of > the > >> > system. > >> > > >> > -- Enrico > >> > > >> > > >> > > >> >> > >> >> > >> >> > > >> >> > -- Enrico > >> >> > > >> >> > > >> >> > 2017-09-13 6:09 GMT+02:00 Yiming Zang <yz...@twitter.com.invalid>: > >> >> > > >> >> > > Sure, I think the current HTTP endpoints in Twitter are only > >> designed > >> >> for > >> >> > > Twitter specific, such as check quorum loss, check rack/region > >> >> diversity. > >> >> > > So the endpoints convention in Twitter are not the same as in the > >> >> > proposal. > >> >> > > I think it would be great to have an agreement on the API naming > >> >> design, > >> >> > so > >> >> > > I like the API design in the proposal, I think the proposal looks > >> >> good to > >> >> > > me. > >> >> > > > >> >> > > Besides, we're currently only using GET functionalities in > Twitter, > >> >> but I > >> >> > > notice there're a lot of POST and PUT APIs in the proposal which > >> could > >> >> > > change the bookie state or trigger some heavy workload. These > APIs > >> >> looks > >> >> > a > >> >> > > bit risky to me if we don't have any authentication enabled (in > >> >> Twitter). > >> >> > > > >> >> > > On Tue, Sep 12, 2017 at 6:11 PM, Sijie Guo <guosi...@gmail.com> > >> >> wrote: > >> >> > > > >> >> > > > + Yiming > >> >> > > > > >> >> > > > Yiming, if you have time, please take a look at this BP. let's > >> see > >> >> if > >> >> > > > there are any conflicts with those you added for autorecovery. > >> >> > > > > >> >> > > > - Sijie > >> >> > > > > >> >> > > > On Tue, Sep 12, 2017 at 8:00 AM, Jia Zhai <zhaiji...@gmail.com > > > >> >> wrote: > >> >> > > > > >> >> > > >> Hi all, > >> >> > > >> > >> >> > > >> Based on Github #278 <https://github.com/apache/boo > >> >> kkeeper/pull/278>, > >> >> > > I > >> >> > > >> have just posted a proposal regarding define BookKeeper public > >> http > >> >> > > >> endpoints: > >> >> > > >> > >> >> > > >> https://cwiki.apache.org/confluence/display/BOOKKEEPER/BP- > >> >> > > >> 17%3A+Define+BookKeeper+public+http+endpoints > >> >> > > >> > >> >> > > >> > >> >> > > >> Github #278 <https://github.com/apache/bookkeeper/pull/278> > >> >> > introduces > >> >> > > >> BookKeeper Http Endpoint module. However there are only two > >> >> endpoints, > >> >> > > >> which is “/heartbeat” and “/api/config/serverconfig”, defined > in > >> >> #278. > >> >> > > In > >> >> > > >> order to fully leverage the http modules, The goal of this BP > >> is to > >> >> > add > >> >> > > >> more endpoints to this modules. > >> >> > > >> > >> >> > > >> Any comments are welcome and appreciated. > >> >> > > >> > >> >> > > >> > >> >> > > >> Thanks. > >> >> > > >> > >> >> > > >> -Jia > >> >> > > >> > >> >> > > > > >> >> > > > > >> >> > > > >> >> > > >> >> > >> > > >> > > >> > > > > -- > > > -- Enrico Olivelli >
