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

The instanceId(use snowflake generator ) is different when register same instance twice #4467

Closed
horizonzy opened this issue Dec 12, 2020 · 13 comments
Labels
Milestone

Comments

@horizonzy
Copy link
Collaborator

@horizonzy horizonzy commented Dec 12, 2020

Describe the bug
Now the user register instance with metadata {"preserved.instance.id.generator", "snowflake"}. It will generate instanceId with auto increment number.
If we register the instance again or update is't metadata in console, it's id will change 0 -> 1 and 1 -> 0 again and again.

Expected behavior
If the origin id is snowflake id, just remains it.

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 14, 2020

If same instance register twice, should different instanceId.

If update instance, the instanceId should be same.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Dec 14, 2020

If same instance register twice, should different instanceId.

If update instance, the instanceId should be same.

We can't differ update or twice register. we modify the instance matadata in console, it's update. But in controller, it's will register again.

Loading

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 14, 2020

So I think just judge whether old instance exist, if exist, use old instanceId, instanceId should not be update except deregister then register.

Or we need to discuss whether push down instance generating to client part. I think id generate in server part is not reasonable.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Dec 14, 2020

So I think just judge whether old instance exist, if exist, use old instanceId, instanceId should not be update except deregister then register.

Or we need to discuss whether push down instance generating to client part. I think id generate in server part is not reasonable.

If generating in client side, we can't gan guarantee that the instanceId will repeat, so also need judge in server side.
It's not a good idea.

Loading

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 14, 2020

Users to guarantee. the instance definition is controlled by users. We only received and apply it.

The snowflake need add an workId, which need user input. It can make sure instance not duplicate.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Dec 14, 2020

Users to guarantee. the instance definition is controlled by users. We only received and apply it.

The snowflake need add an workId, which need user input. It can make sure instance not duplicate.

what's the concept of workId?

Loading

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 14, 2020

this is a concept from snowflake.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Dec 14, 2020

this is a concept from snowflake.

good idea, but it will increase the cost of use for user, user need consider the way how to differ workId. It's unfriendly for user.

Loading

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 15, 2020

Even add it in server part, user also need to learn workid and snowflake, otherwise they can't use well because User includes developer and maintainer.

BTW, the instanceId means the identify of instance, not nacos identify. So I think generate by client with app is more reasonable.

Also we can generate the workId in client part, such as use ip or hostname to hash an workId as default if users do not config.

Loading

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 15, 2020

But now we has implemented in server part, so we need to adapt it first in a few time.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Dec 15, 2020

Also we can generate the workId in client part, such as use ip or hostname to hash an workId as default if users do not config.

hash conflict need be consider, it also cann't guarantee the unique workId.

Loading

@horizonzy
Copy link
Collaborator Author

@horizonzy horizonzy commented Dec 15, 2020

workId may not suit our situation, the workId is certain such as 2^n, we know the value of n. But we don't know the number of computer will register instance to server.

Loading

@KomachiSion
Copy link
Collaborator

@KomachiSion KomachiSion commented Dec 15, 2020

I just give an example, when we execute this plan, we need to discuss more details.

Loading

KomachiSion pushed a commit that referenced this issue Dec 15, 2020
* fix snowflake id problem

* revert code format
@KomachiSion KomachiSion added this to the 1.4.1 milestone Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants