Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clients in other languages #11

Closed
ghost opened this issue May 16, 2019 · 33 comments
Closed

clients in other languages #11

ghost opened this issue May 16, 2019 · 33 comments

Comments

@ghost
Copy link

ghost commented May 16, 2019

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
Copy link
Member

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
Copy link

Is there a nodejs client?

@tylertreat
Copy link
Member

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
Copy link

@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
Copy link
Member

@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
Copy link

joeblew99 commented Aug 28, 2019 via email

@paambaati
Copy link

@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
Copy link
Member

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
Copy link

paambaati commented Sep 3, 2019

@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
Copy link
Member

@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:

go-liftbridge/client.go

Lines 591 to 593 in 6fbf530

// The server will either send an empty message, indicating the
// subscription was successfully created, or an error.
_, err = st.Recv()

@paambaati
Copy link

paambaati commented Sep 3, 2019

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
Copy link
Member

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
Copy link

@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
Copy link
Member

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
Copy link

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
Copy link
Member

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

@paambaati
Copy link

paambaati commented Sep 7, 2019

@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
Copy link

dgzlopes commented Sep 7, 2019

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
Copy link

dgzlopes commented Sep 7, 2019

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
Copy link

paambaati commented Sep 7, 2019

@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 #11 (comment)) 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
Copy link

@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
Copy link

dgzlopes commented Sep 7, 2019

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
Copy link

@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
Copy link

@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
Copy link
Member

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
Copy link

paambaati commented Sep 9, 2019

@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
Copy link

paambaati commented Sep 9, 2019

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
Copy link

paambaati commented Sep 10, 2019

@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
Copy link
Member

@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
Copy link

@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
Copy link

paambaati commented Sep 17, 2019

@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
Copy link
Member

@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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants