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

Delayed synchronization upon frontend server restart #48

Closed
semin-park opened this issue Aug 1, 2020 · 5 comments · Fixed by #63
Closed

Delayed synchronization upon frontend server restart #48

semin-park opened this issue Aug 1, 2020 · 5 comments · Fixed by #63
Labels
bug 🐞 Something isn't working good first issue 🐤 Good for newcomers

Comments

@semin-park
Copy link
Contributor

semin-park commented Aug 1, 2020

What happened

While running the toy example (docker-compose up => npm start) I noticed that if I restart the frontend server, synchronization happens only after a client makes some local changes to the document. So for example if I make changes on a client, the changes are not reflected in any other clients until they create some events on their part (e.g., moving the cursor around, writing to the document et cetera)

What you expected to happen: Seamless synchronization

How to reproduce it (as minimally and precisely as possible):

  1. Run docker-compose up in docker/
  2. Run npm start
  3. Quit the frontend server with a keyboard interrupt (Ctrl + C)
  4. Again run npm start
  5. Make any changes to a client document and notice that the change is not going to propagate to other clients until they do something.

Anything else we need to know?:
When I quit the frontend server, it does show an error: Assertion failed: (0), function uv_close, file ../deps/uv/src/unix/core.c, line 178.

Then after restarting the server, it produces this error whenever I close a client browser tab: [HPM] Error occurred while trying to proxy request /api.Yorkie/WatchDocuments from localhost:9000 to http://localhost:8080 (ECONNRESET) (https://nodejs.org/api/errors.html#errors_common_system_errors)

Environment:

  • Operating system: macOS Catalina 10.15.6
  • Browser and version: Chrome, Version 84.0.4147.105 (Official Build) (64-bit)
  • Yorkie version (use yorkie version): Yorkie: 0.0.10 (Commit: 165cdbf / Go: go1.14.3)
  • Yorkie JS SDK version: 0.0.9
@ppeeou
Copy link
Member

ppeeou commented Aug 2, 2020

It has to do with this issue yorkie-team/yorkie#69

@hackerwins hackerwins added the bug 🐞 Something isn't working label Aug 12, 2020
@hackerwins
Copy link
Member

hackerwins commented Aug 13, 2020

This is related to the issues below. I think we need to update gRPC-Web to 1.2.0.

grpc/grpc-web#450
grpc/grpc-web#858

https://github.com/yorkie-team/yorkie-js-sdk/blob/master/src/core/client.ts#L419-L423

@hackerwins hackerwins added the good first issue 🐤 Good for newcomers label Aug 13, 2020
@hackerwins
Copy link
Member

There was a problem that error events were missing in the previous version of grpc-web.

It seems to have been fixed in 1.2.0, which was released a while ago. I think we should check it again. And grpc-web network error handling in 1.2.0 is described in the table in the issues below.

grpc/grpc-web#858 (comment)

So I think we can solve this problem like this:

  1. update grpc-web to 1.2.0
  2. catch multiple error conditions(see grpc-web table above) and handle it with retry logic in below

https://github.com/yorkie-team/yorkie-js-sdk/blob/master/src/core/client.ts#L419-L423

@hackerwins
Copy link
Member

@semin-park

Also, I found a bug in pubsub module that only occurs under certain conditions. It was easy to reproduce if I tested it the way you report. This report is very helpful.

https://github.com/yorkie-team/yorkie/blob/master/yorkie/pubsub/pubsub.go

Thanks.

@semin-park
Copy link
Contributor Author

semin-park commented Aug 20, 2020

A couple comments on what I've tried on my local repo:

  1. I've upgraded grpc-web to version 1.2.0 and yorkie to 0.0.11
  2. I've changed the rpcAddr of the js client to point to the envoy proxy directly, instead of the frontend server (from /api to localhost:8080 where envoy lives)
    const client = yorkie.createClient('/api');

Results

  • Now the frontend server going down doesn't create any problems, as expected
  • When envoy goes down, connection is lost (as expected), and .on('error', ...) does get triggered
    • This was not the case when the js client was pointing to /api.
  • But even when envoy comes back up, connection doesn't get reestablished automatically -- this needs to be fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working good first issue 🐤 Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants