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

Communication issue between services with v1.0.2 #70

Closed
dekelev opened this issue Jun 21, 2020 · 10 comments
Closed

Communication issue between services with v1.0.2 #70

dekelev opened this issue Jun 21, 2020 · 10 comments
Labels
awaiting response Awaiting more information question

Comments

@dekelev
Copy link

dekelev commented Jun 21, 2020

We have a communication issue between services in different apps with feathers-distributed v1.0.2

  • I have 2 apps. app1 needs to call a remote service in app2
  • I deployed app1
  • I deployed app2
  • app1 registers a new remote service from app2. the RemoteService class is initiated with the service path and the serviceDescriptor (that has the remote app UUID) as a context.
  • Second app restarts.
  • app1 is notified when app2 starts, only that now app1 will ignore the remote service from app2 since it was already registered. it seems that app1 only checks the path for a match, while the UUID of the remote app is ignored.
  • Calls from app1 to the remote service in app2 now reaches timeout every time.

How does it work or supposed to work? what am I missing?

@claustres
Copy link
Member

I think you are right because by default the distribution key is the app uuid so that if you want your service namespace to be persistent even on app restart you have to assign a specific key to your app like this:

app.configure(distribution({
  key: 'mykey'
}))

Could you please check if it works better ? Will try to make this more clear in docs anyway.

Thanks for feedback.

@claustres claustres added awaiting response Awaiting more information question labels Jun 22, 2020
@dekelev
Copy link
Author

dekelev commented Jun 22, 2020

Thanks @claustres , I'll was able to reproduce this issue locally very easy by restarting app2.

We tried upgrading from v0.7.1 to this version on a staging environment and communication between services failed after every re-deployment.

It seems to works well after setting the key to app1 in app1 and app2 in app2. it even works when changing Cote base-port between app2 restarts.

When checking the logs, it seems that app1 is still ignoring remote services notifications from new instances of app2, but all works.

I think that without setting the key option to a static value, the environment will always be broken. I'm also not sure how it works with multiple instances of app2.

Can you please explain how this work? Should I set the same static value on the key option in all the apps or key should be unique for each app?

@claustres
Copy link
Member

I've just set a default key for all apps so that by default this should work fine, although there is no partition. Please reopen if required, otherwise we will publish a patch.

@dekelev
Copy link
Author

dekelev commented Jun 22, 2020

Thank you!

@dekelev
Copy link
Author

dekelev commented Jun 28, 2020

@claustres The solution did not work well. we got the following error when trying to call services between apps:

GeneralError: [object Object]
    at new GeneralError (.../node_modules/@feathersjs/errors/lib/index.js:170:17)
    at convert (.../node_modules/@feathersjs/errors/lib/index.js:239:7)
    at .../node_modules/@kalisio/feathers-distributed/lib/service.js:43:35
    at Generator.throw (<anonymous>)
    at step (.../node_modules/@kalisio/feathers-distributed/lib/service.js:15:191)
    at .../node_modules/@kalisio/feathers-distributed/lib/service.js:15:402
    at process._tickCallback (internal/process/next_tick.js:68:7)

From Cote:
"Keys are useful in this scenario: requesters and responders around service A and messages X and Y should use one particular key, and requesters and responders around service B and messages Z and T should use another key. In this case, no messages will be lost, and the services will be segregated."

As I understand this, service A in this example is a Feathers app, so it needs a different key than the other Feathers app in the Cote discovery environment.

I think the issue is that every app needs to have a unique & consistent distribution key.

All works well after setting the distribution key to the package.json's name field. for us, the app name is unique & consistent value per app.

@claustres
Copy link
Member

Yes usually you should have a different key per app if you have multiple apps and each app manages different services, that's the reason why we previously used the app uuid as a key but this is not consistent across restart.

Not sure what is the best default as it really depends on the use case, eg you can also have multiple but replicated apps owing similar services.

@dekelev
Copy link
Author

dekelev commented Jun 29, 2020 via email

@claustres
Copy link
Member

I was talking about launching the same app with the same set of services multiple times in order to scale and load balance requests between each app not about duplicating the same services across different apps. I agree the package.json name field is a solid solution because duplicated apps will then have the same key while different projects will not, and it will be persistent across restart. However I am somewhat reluctant to hard code this in the library, people may want to use more "domain driven" names to group services. I will try to update the docs in this way unless you want to PR the docs if you already have an idea about how to explain this.

@dekelev
Copy link
Author

dekelev commented Jun 29, 2020

Thanks! I don't have any suggestion to the docs, besides highlighting the info since it's a critical option and the lib does not work as intended without it.

I'm setting the key option with this:

const appRoot = require('app-root-path');
const pjson = require(`${appRoot.path}/package.json`);

app.configure(distributed({
  ...,
  key: pjson.name
}));

@claustres
Copy link
Member

Just enhanced the doc with this https://github.com/kalisio/feathers-distributed/blob/master/README.md#partition-keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response Awaiting more information question
Projects
None yet
Development

No branches or pull requests

2 participants