Skip to content

EncodingProviders must use abs(machine id) #5

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

Open
jonross opened this issue Mar 25, 2015 · 3 comments
Open

EncodingProviders must use abs(machine id) #5

jonross opened this issue Mar 25, 2015 · 3 comments

Comments

@jonross
Copy link

jonross commented Mar 25, 2015

Negative machine IDs are possible, and SnowflakeEncodingProvider left-shifts these without checking, resulting in a small negative number that is OR'ed with the timestamp, erasing the timestamp. Example:

    static class FixedIdProvider implements MachineIdProvider {
        final long id;
        FixedIdProvider(long id) {
            this.id = id;
        }
        public long getMachineId() {
            return id;
        }
    }

    public static void main(String[] args) throws Exception
    {
        IdGenerator g1 = newSnowflakeIdGenerator(new FixedIdProvider(1));
        System.out.println(g1.generateId(0).asString());
        IdGenerator g2 = newSnowflakeIdGenerator(new FixedIdProvider(-1));
        System.out.println(g2.generateId(0).asString());
    }

Output:

080f6b18b1001000
fffffffffffff000

Creating a wrapper on e.g. MacPidMachineIdProvider is an easy temporary workaround.

jonross added a commit to jonross/fauxflake that referenced this issue May 12, 2015
@abdielou
Copy link

@rholder Is there an ETA on this pull?
@jonross There's another pull from @azderski with a different implementation but similar issue. Is it a different problem?

@jonross
Copy link
Author

jonross commented Jul 24, 2015

It looks like the other implementation fixes the MAC address value, however another machine ID implementation could still return < 0. Also I included a test.

@abdielou
Copy link

Good. Then I'll keep using your branch while this gets merged.

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

2 participants