liftbridge-io / go-liftbridge

Go client for Liftbridge. https://github.com/liftbridge-io/liftbridge
Apache License 2.0
68 stars 18 forks source link

clients in other languages #11

Closed ghost closed 5 years ago

ghost commented 5 years ago

From the looks of things most of the code is GRPC code generated.

Would be cool to setup a docker to make the code for java, nodejs, Dart, etc. Then i am guessing some people that use that language can write the extra code needed by looking at how simple it is in the golang code.

I think the Dart one would be pretty easy as i have done some GRPC with Dart. That would make Liftbridge work with Flutter.

Just noticed a ruby one:https://github.com/andyl/liftbridge_ruby

is there an Awesome Liftbridge list anywhere?

tylertreat commented 5 years ago

Yeah, more language clients are on my radar, just haven't gotten to it yet (plus there's a lot of server stuff to do). I plan to do a Python and Java client next if someone else doesn't get to it first. Dart would be great too. Generating the gRPC code is actually very straightforward, I just need to get it properly documented. And like you said, there is pretty minimal code on top of what's provided by gRPC.

canadaduane commented 5 years ago

Is there a nodejs client?

tylertreat commented 5 years ago

Not at the moment, sorry. Generating the gRPC bindings give you a low-level client though.

https://github.com/liftbridge-io/liftbridge-grpc

I haven't had time to document this yet.

joeblew99 commented 5 years ago

@tylertreat

Saw the python client and his notes about what extra code a client needs apart from grpc. Maybe its getting to the point that a MarkDown can be put on the main repo for client implementers to get an idea of the effort ?

https://github.com/dgzlopes/python-liftbridge#how-to-contribute

tylertreat commented 5 years ago

@joeblew99 Agreed, I think it's to the point where we can document the steps for creating a client library. I'm planning to merge this this week which implements stream partitioning (and should be the last major API change before a stable release). Once this is merged I'll add some markdown detailing what's needed to implement a client. Maybe @dgzlopes can provide some input based on his work with the Python client.

joeblew99 commented 5 years ago

Wow exciting.

Looks like it might finally be ready for action.

I am planning to put a web gateway on top to make it easy to build front-end.

https://github.com/dunglas/mercure

https://gitlab.com/wallforfry/dart_mercure

The way it works is to use SSE to push updates to front ends

Anyways just figured it is worth mentioning.

On Wed, 28 Aug 2019, 17:28 Tyler Treat, notifications@github.com wrote:

@joeblew99 https://github.com/joeblew99 Agreed, I think it's to the point where we can document the steps for creating a client library. I'm planning to merge this https://github.com/liftbridge-io/liftbridge/pull/93 this week which implements stream partitioning (and should be the last major API change before a stable release). Once this is merged I'll add some markdown detailing what's needed to implement a client. Maybe @dgzlopes https://github.com/dgzlopes can provide some input based on his work with the Python client.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/liftbridge-io/go-liftbridge/issues/11?email_source=notifications&email_token=AC3RU45RDFHMBI2V6EOW3ULQG2KQJA5CNFSM4HNM7ONKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5LQM6A#issuecomment-525796984, or mute the thread https://github.com/notifications/unsubscribe-auth/AC3RU46YKB5CSFTKWW5DO7DQG2KQJANCNFSM4HNM7ONA .

paambaati commented 5 years ago

@tylertreat I’d love to build a Node.js client!

A quick note — it would be help if there were releases with changelogs so we could start tracking changes.

tylertreat commented 5 years ago

Yeah, I plan to start doing releases now that there shouldn't be any more breaking changes to the API. Will definitely include changelogs.

paambaati commented 5 years ago

@tylertreat Sounds great! I've started work on the Node.js client (not public yet), but I've run into some issues (specifically, a code 13 "failed to parse server response" error). What would be a good place to ask for help?

EDIT

I have a repro now —

  1. docker run -p 4222:4222 -ti nats:latest --debug --trace in a window.
  2. go get github.com/liftbridge-io/go-liftbridge and then $GOPATH/bin/liftbridge --raft-bootstrap-seed --nats-servers nats://localhost:4222 --level debug in another window.
  3. Clone my repo from https://github.com/paambaati/node-liftbridge.git in yet another window.
    1. yarn install or npm install
    2. yarn run debug or npm run debug

The debug script attempts to create a new stream, then publish a few messages, then subscribes to the same stream (subject) and then publishes a few more messages.

Expected output — Each published message should be printed to console (see relevant line).

Actual output —

Error: 13 INTERNAL: Failed to parse server response
    at Object.exports.createStatusError (~/node-liftbridge/node_modules/grpc/src/common.js:91:15)
    at ClientReadableStream._emitStatusIfDone (~/node-liftbridge/node_modules/grpc/src/client.js:233:26)
    at ClientReadableStream._receiveStatus (~/node-liftbridge/node_modules/grpc/src/client.js:211:8)
    at Object.onReceiveStatus (~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:1272:15)
    at InterceptingListener._callNext (~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:568:42)
    at InterceptingListener.onReceiveStatus (~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:618:8)
    at ~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:1029:24 {
  code: 13,
  metadata: undefined,
  details: 'Failed to parse server response'
}
tylertreat commented 5 years ago

@paambaati It looks like it's failing on subscribe? I bet it's because Liftbridge first sends an empty message on the stream when a subscription is successfully created (or an error if not). Are you handling this case in the client? See the Go client for an example: https://github.com/liftbridge-io/go-liftbridge/blob/6fbf530bb220797fd91174d9f858fad3114dbc48/client.go#L591-L593

paambaati commented 5 years ago

Liftbridge first sends an empty message on the stream

I’m assuming that empty message conforms to the proto.

@tylertreat My initial guess is the response couldn’t be parsed by gRPC, and hence the error code 13 (it is reserved only for serious errors).

I’m gonna debug this externally with tools and see if I’m able to reproduce this.

tylertreat commented 5 years ago

I’m assuming that empty message conforms to the proto.

Yes, it is a normal proto message, so I'm not sure why you'd be hitting a parse error. I will see if I can spend some time looking at it today.

paambaati commented 5 years ago

@tylertreat After studying the Go client implementation more closely, I’ve also realized there’s a lot more I’ve yet to implement (for example, partitioners) and things I haven’t fully understood (like subjects and how they can have a .<partition> notation). I’m gonna pause work on the Node.js client I can understand the client semantics more.

Documentation about the key operations (publish, subscribe, fetch metadata, etc.) and how to implement clients would be super-helpful. If there’s a branch where they’re being worked on, I’ll be more than happy to contribute. I’m currently grokking more of the Go client codebase and plan to write down my version of a “How to write a Liftbridge client” too.

tylertreat commented 5 years ago

Yep, I'm planning to write up a guide on implementing a client later this week. Partitioning was just merged. The client doesn't actually have to support it to start using Liftbridge since when you create a stream it consists of just a single partition by default, so publish and subscribe work as normal without a partitioner.

paambaati commented 5 years ago

when you create a stream it consists of just a single partition by default

A-ha! I didn’t know this. Would be great if the documentation also marked the fields as mandatory or optional.

tylertreat commented 5 years ago

@paambaati I'm starting to sketch out a guide for implementing client libraries here. Still a work in progress...

paambaati commented 5 years ago

@tylertreat Looks great! Thanks for writing it. If I may, I have some feedback around it —

  1. Can you clarify the subject naming when there’s more than 1 partitions? From the Go code, what I understand is that if there’s more than 1 partition, the subject is changed to <subject>.<partition>. Is this correct? Should we perhaps explicitly call this out?

  2. The message envelope must be documented explicitly, IMO. I see that we’re prefixing messages with LIFT on publish, but I don’t see us using that as a message boundary in subscribe, and I’m still not sure how/why it works.

  3. How does one subscribe to other partitions? The default is to subscribe to partition 1; does the client subscribe to other partitions automatically?

dgzlopes commented 5 years ago

Love the shape the guide is taking! Also, one thing that I have on the Python client backlog is the usage of a connection pool. Actually, from the code docs:

Multiple addresses can be provided. Connect will use whichever it connects successfully to first in random order. The Client will use the pool of addresses for failover purposes. Note that only one seed address needs to be provided as the Client will discover the other brokers when fetching metadata for the cluster.

Maybe a further explanation on the guide would be helpful. Also, now that docs are getting bigger and new libraries are being developed, migrating them to some central location using Mkdocs (or something similar) and CD/CI would be great. It would make the Liftbridge readme smaller too!

On the other hand, having a release system would be really helpful for automatically building docker images [0]

[0] https://github.com/dgzlopes/liftbridge-docker

dgzlopes commented 5 years ago

Also, I think defining a standard way to consume the bindings might be useful. Actually, go client uses the liftbridge-grpc [0] ones, the python/node clients builds them on fixed commit basis [1][2] and other clients even have the protos inside the repo [3].

One solution might be to use git submodules or to ship the bindings as a different package on each language, but I think this topic needs further discussion. From researching other projects using gRPC, seems like they are all doing it differently... So I think we should pick the approach that works the best for Liftbridge.

[0] https://github.com/liftbridge-io/liftbridge-grpc [1] https://github.com/dgzlopes/python-liftbridge/blob/9e5f1b47eb354e3eaf4b8faf1b729c5aa554f3e7/Makefile#L1 [2] https://github.com/paambaati/node-liftbridge/blob/master/scripts/generate_grpc_code.sh [3] https://github.com/andyl/liftbridge_ruby

paambaati commented 5 years ago

@tylertreat @dgzlopes I'm still struggling with getting Publish and Subscribe working correctly over at node-liftbridge. Would you mind taking a look at it?

The debug script (shared earlier in https://github.com/liftbridge-io/go-liftbridge/issues/11#issuecomment-527367421) publishes messages, but the responses always have an empty ack, and subscribe always returns a { code: 13, details: 'Failed to parse server response' } error.

For example, here's a Message I'm publishing —

{
  offset: 0,
  key: 'S0VZLTVjZmE1NTk1ZTRmNDJjMWQyYmEw',
  value: 'VALUE-ok-KEY-5cfa5595e4f42c1d2ba0',
  timestamp: 1567847326240000000,
  subject: 'test7',
  reply: '',
  headersMap: [],
  ackinbox: 'test7.acks',
  correlationid: 'd259fd66-c423-49c3-88d8-bb03fff95a4d',
  ackpolicy: 2
}

and here's the ack I get every time —

{ ack: undefined }

I'm fairly certain I'm doing something wrong in writing the payload or the subscribe semantics (because the code that the Go client handles on subscribe is FAILED_PRECONDITION, while the code I'm facing is INTERNAL - see https://github.com/grpc/grpc/blob/master/doc/statuscodes.md), but I'm still not sure yet. Any help would be appreciated.

paambaati commented 5 years ago

@dgzlopes About the gRPC bindings, I use a method very similar to yours — see https://github.com/paambaati/node-liftbridge/blob/master/scripts/generate_grpc_code.sh

dgzlopes commented 5 years ago

Oops! My bad @paambaati, I will edit my latest comment. I saw the proto file being tracked and I though It was hardcoded on the repo.

Also, I'm actually diving on your issue. Even if my skills on ts are really low right now I will try to identify anything gRPC/Liftbridge related that seems strange.

paambaati commented 5 years ago

@dgzlopes Thanks a lot!

In the meanwhile, I ran a detailed trace, and here's what I get - https://gist.github.com/paambaati/7884b119eee47fafa436f74db8b59edc

I'm grasping at straws here but I'm hoping this trace gives us some clue.

I've padded the debug script's output with a lot of new lines so it is a little easier to read.

paambaati commented 5 years ago

@tylertreat @dgzlopes More debugging information!

I tried running my debug script to publish some data and then subscribing to the same stream/subject using the example Go client, and turns out, it works (🥳) and I can see my data on the Go side.

tylertreat commented 5 years ago

Sorry, I've been out of the office. Lots of threads of discussion here. :)

Thanks for the feedback on the client implementation guide. I will make sure to include it!

Also, now that docs are getting bigger and new libraries are being developed, migrating them to some central location using Mkdocs (or something similar) and CD/CI would be great. It would make the Liftbridge readme smaller too!

@dgzlopes Totally agree. I'd like to set up a nicer docs solution.

Also, I think defining a standard way to consume the bindings might be useful.

Yep, open to discussion on that. I've been generating code in the liftbridge-grpc repo for use in libraries (just Go and Python right now), but not sure if that's the best approach or not.

@paambaati I will try to take a look at your issue this week. Based on your most recent message, it sounds like publish might be working at least? Or is there still an issue with the ack?

paambaati commented 5 years ago

@tylertreat The ack is still undefined, sadly. But the subscribe via your Go client can read the messages published by my Node.js client.

And about the client implementation guide, here’s what would be nice to have too —

  1. Strategies for implementing a connection pool.

  2. The resubscribe logic (and why we need it in the first place).

These can probably be separated into a separate section that talks about higher-level utility functions while the rest can be called the low-level wire protocol or the basic communication APIs.

paambaati commented 5 years ago

And for the documentation, I’d suggest evaluating Docusaurus too. It is developed by Facebook and used by a lot of projects, and IMHO, looks really nice.

I can also help with proof-reading the documentation and creating custom stylesheets for the docs to fit the Liftbridge theme.

paambaati commented 5 years ago

@tylertreat @dgzlopes I was finally able to narrow it down to the root cause! You can follow the full issue here.

tl;dr

The server responds to Subscribe with a response like this —

key:"some-key" value:"some-value" timestamp:1568100468661514000 subject:"test11" headers:<key:"reply" value:"" > headers:<key:"subject" value:"test11" >

Note the headers being a map, and the reply header's value being an empty string. node-grpc (or perhaps google-protobuf) reads the reply header's value as undefined, which was throwing an assertion failure.

@tylertreat Is the reply header expected? What does it actually do? What does the value being an empty string mean? Do you have any ideas around how to fix this?

tylertreat commented 5 years ago

@paambaati Yes, reply is a built-in header set by Liftbridge. It is the NATS reply-to subject sent on the published NATS message (see https://nats-io.github.io/docs/nats_protocol/nats-protocol.html#pub).

Also, I've started looking into the ack issue with publishes. I've been trying to wrap my head around the Typescript gRPC API, but I'm thinking the ack is undefined because Liftbridge only sends an ack if a gRPC timeout/deadline is set on the publish.

paambaati commented 5 years ago

@tylertreat @dgzlopes FWIW, I've narrowed the issue much further down to protocolbuffers/protobuf-javascript#43

While I can monkey-patch the fix, I'd rather not and I'm hoping we come up with a fix soon over at the protobuf repo.

In the meantime, I've finished implementing most of the methods (connection pooling and unit tests are left) and also have documentation up and running — https://paambaati.github.io/node-liftbridge/globals.html 🎉

paambaati commented 5 years ago

@tylertreat I'm starting work on the connection pool and resubscribe logic, and I'm trying to understand your implementation. I have some questions if you don't mind answering them.

  1. The dispatchStream method tries to resubscribe on "Unavailable" errors. How does one go about testing this? I tried SIGINTing the liftbridge server, and I see 2 different kinds of responses.

    For example, the Go subscribe client returns —

    panic: rpc error: code = Unavailable desc = transport is closing
    
    goroutine 41 [running]:
    main.main.func1(0x0, 0x15b7760, 0xc0001d2410)
        ~/go/src/github.com/liftbridge-io/go-liftbridge/example/lift-sub/subscribe.go:35 +0x3c8
    github.com/liftbridge-io/go-liftbridge.(*client).dispatchStream(0xc0001600a0, 0x15c05c0, 0xc0000dc008, 0x151d3ea, 0x11, 0x15c38c0, 0xc0001f9280, 0xc0000f07c0, 0x153a558)
        ~/go/src/github.com/liftbridge-io/go-liftbridge/client.go:662 +0x376
    created by github.com/liftbridge-io/go-liftbridge.(*client).Subscribe
        ~/go/src/github.com/liftbridge-io/go-liftbridge/client.go:464 +0x18e
    exit status 2

    Meanwhile in Node-land, I get this —

    Error: 2 UNKNOWN: Stream removed
      at Object.exports.createStatusError (~/node-liftbridge/node_modules/grpc/src/common.js:91:15)
      at ClientReadableStream._emitStatusIfDone (~/node-liftbridge/node_modules/grpc/src/client.js:233:26)
      at ClientReadableStream._receiveStatus (~/node-liftbridge/node_modules/grpc/src/client.js:211:8)
      at Object.onReceiveStatus (~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:1272:15)
      at InterceptingListener._callNext (~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:568:42)
      at InterceptingListener.onReceiveStatus (~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:618:8)
      at ~/node-liftbridge/node_modules/grpc/src/client_interceptors.js:1029:24 {
    code: 2,
    metadata: Metadata { _internal_repr: {}, flags: 0 },
    details: 'Stream removed'
    }

    Note that I get an error code 2 (Unknown) in Node.js while I get Unavailable as expected in Go. Is this yet another Node.js-specific quirk?

  2. I'll keep adding more as I go to this issue. Is this okay, or would you rather I open a new issue?

tylertreat commented 5 years ago

@paambaati

Note that I get an error code 2 (Unknown) in Node.js while I get Unavailable as expected in Go. Is this yet another Node.js-specific quirk?

Interesting, I would expect gRPC clients to behave the same in terms of error codes. Not sure if that is unique to the Node implementation. My testing approach for the Go client was to simply kill and restart the server as you indicated. Perhaps it's appropriate for clients to handle this error case as well? At any rate, the resubscribe logic is purely best-effort resiliency. At the moment, it's on clients to track their position in the log if needed. Next up after I finish the client guide documentation work I will be implementing "consumer groups" which will have capabilities for automatically tracking log position similar to Kafka.

I'll keep adding more as I go to this issue. Is this okay, or would you rather I open a new issue?

If there are items specific to the Node client, it might be easier to track discussion in a separate issue? Perhaps we can use this thread for discussing the in-progress client guide documentation (which I'm planning to return to later this week).

tylertreat commented 5 years ago

Closing this issue with the introduction of client implementation docs. I've also created a separate issue for migrating docs to a proper documentation system.