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

constant 3735928559 overflows int #68

Closed
jaeyeom opened this issue Aug 24, 2014 · 43 comments
Closed

constant 3735928559 overflows int #68

jaeyeom opened this issue Aug 24, 2014 · 43 comments

Comments

@jaeyeom
Copy link

jaeyeom commented Aug 24, 2014

github.com/HouzuoGuo/tiedot/data/hashtable.go:43: constant 3735928559 overflows int

I am compiling tiedot from ARM based Linux Raspbian on my Raspberry Pi and I got the above message. Go version 1.3.

@jaeyeom
Copy link
Author

jaeyeom commented Aug 24, 2014

Ah, I should have read comments in the code. It's a known issue.

@HouzuoGuo
Copy link
Owner

Take a look at:
https://github.com/HouzuoGuo/tiedot/blob/master/buildconstraint.go
It surprises me that buildconstraints did not prevent building on 32-bit ARM processor at first place. If it had worked, you would have been prompted with this known issue, and a suggested workaround.

In your first attempt, did you get a build failure caused by buildconstraint.go?

@jaeyeom
Copy link
Author

jaeyeom commented Aug 26, 2014

Ah, if I go build buildconstraint.go, it does produce error. But when I did "go get github.com/HuozuoGuo/tiedot", I couldn't see any other lines than overflows int.

Anyway, thank you very much. I'll close this bug.

@jaeyeom jaeyeom closed this as completed Aug 26, 2014
@HouzuoGuo
Copy link
Owner

that's very interesting to know, thanks for the report!

@HouzuoGuo
Copy link
Owner

By the way - please also try out "32bit" branch. It is a slightly modified version of current-master, specially designed for 32-bit embedded/ARM systems.

@gibsonsyd
Copy link

Can you provide a link to the "32bit" branch please?

@HouzuoGuo
Copy link
Owner

oops... sorry I was cleaning up the branches and accidentally removed the 32-bit branch. But don't worry - I'll make a new 32-bit cut tomorrow, it will be fully tested, and will work as good as the original 32-bit branch.

@HouzuoGuo HouzuoGuo reopened this Dec 3, 2014
@HouzuoGuo
Copy link
Owner

Check out 32bit branch and refer to the documentation in master branch.

It is a cut of working-in-progress "singlethread" branch. therefore the database is not compatible with master branch or 3.x release.

@gibsonsyd
Copy link

Thank you! I'll give it a go!

@HouzuoGuo
Copy link
Owner

How's the 32-bit branch working for you @gibsonsyd ?

@gibsonsyd
Copy link

Hey Howard,

I have very specific needs for sorted queries on date. Tiedot is
unfortunately a little short on the query end for me. It might be my
limited knowledge rather than your api that limits my ability here but time
is of the essence. I've had to look at alternatives for more query options.

The 32bit version does however compile and play well with ARM. I did
partially implement a db in tiedot and managed to deploy it to an ARM
device with no issues.

I'd love to see more advanced queries in Tiedot. I do like it.

Regards

On Mon, Dec 8, 2014 at 3:40 PM, Howard notifications@github.com wrote:

How's the 32-bit branch working for you @gibsonsyd
https://github.com/gibsonsyd ?


Reply to this email directly or view it on GitHub
#68 (comment).

@HouzuoGuo
Copy link
Owner

All right, I'll keep you informed once more query options become available.

@ktw
Copy link
Collaborator

ktw commented Nov 17, 2015

Hi.

Will this 32 bit branch be updated to version 3.2 at some point?
I am trying to cross compile for a Scaleway C1 instance. (ARMv7 Cortex-A9)

@HouzuoGuo
Copy link
Owner

hello @ktw, actually there isn't a big difference between 32bit branch based on 3.1.3 and the latest release 3.2.

3.2 introduces optionally enabled JWT for authorizing API calls, it fixes a performance issue observed on certain OS/FS combinations, and removes web control panel.

Is there a feature you need from 3.2? I can backport it for you if you like.

@mmindenhall
Copy link
Collaborator

I was also using tiedot in a prototype earlier in the year, and it looked to me like the 32-bit branch would not be needed if a few int declarations were simply changed to int64 in the codebase. If that's the case, why not do that instead of backport and maintain a separate branch?

@HouzuoGuo
Copy link
Owner

@mmindenhall that's a good point, indeed I should have done that from the very beginning.
all right, let's do it this way in master.

@HouzuoGuo HouzuoGuo reopened this Nov 17, 2015
@davidmcclelland
Copy link

Any updates on this? I understand that it's probably low priority but I'm running into it as well.

@HouzuoGuo
Copy link
Owner

Hello WinterPhoenix. Could you please give "32bit" branch a try and see if that one works for you?

@davidmcclelland
Copy link

Sure, I expect that the 32bit branch should do what I need it to do. I just wanted to see if a fix for this was imminent before I spent time switching to the branch. I saw that it was reopened in November and figured I'd check. Thanks!

@archey
Copy link
Collaborator

archey commented Feb 3, 2017

Any update on this issue? It would be nice if this could be fixed.

@HouzuoGuo
Copy link
Owner

@archey very sorry about this, it's been an outstanding bug for nearly three years already.

I have trouble finding a 32-bit system to run some tests, if you could do me a small favour, please try this quick & dirty workaround - remove the entire "interger-smear" section from https://github.com/HouzuoGuo/tiedot/blob/master/data/hashtable.go#L39 and run its tests. The hash table will appear somewhat unbalanced, but should remain functional. If that workaround helps, the function HashKey should be elevated into its own arch depent source file.

CC @gertcuykens @davesters @omeid @Dr-Terrible @Komosa & all collaborators - what do you recommend?

@archey
Copy link
Collaborator

archey commented Feb 7, 2017

@HouzuoGuo Ill spin up a 32bit vm and test it out and let you know.

@archey
Copy link
Collaborator

archey commented Feb 10, 2017

@HouzuoGuo is there any reason why you cant change the int to uint64 that would allow it to run 32bit systems?

@HouzuoGuo
Copy link
Owner

You're right, that should have been done from the very beginning and there's no good reason against it. In fact I began experimenting with it - along with a home grown RPC protocol, in a separate branch (https://github.com/HouzuoGuo/tiedot/blob/singlethread/data/partition.go).

But altering it on master branch would be risky, there will be a lot of compilation errors caused by incompatible types.

Now I regret not having done so from very beginning, even more.

@archey
Copy link
Collaborator

archey commented Feb 10, 2017

@HouzuoGuo I have it compiling on arm and 386 by moving that hashkey func to its own go file like doing hash32.go and hash64.go then using // +build params to build the specific version of arm or 386 but not build the 64bit version. I had to remove the buildconstraint.go temporarily to get it to build. I started to run the tests, but my rootfs on this box is only a few gigs so it ran out of space.

@HouzuoGuo
Copy link
Owner

thanks very much for the feedback.

the other thing I regret is that tiedot allocates disk space way too aggressively, since you have a 32-bit environment handy for testing, would you mind to also tweak these constants:

https://github.com/HouzuoGuo/tiedot/blob/master/data/collection.go#L24
Change COL_FILE_GROWTH to 8 * 1048576

https://github.com/HouzuoGuo/tiedot/blob/master/data/hashtable.go#L22

  • Change HT_FILE_GROWTH to 8 * 1048576
  • INITIAL_BUCKETS to 16384
  • HASH_BITS to 14

Looking forward to your feedback!

@archey
Copy link
Collaborator

archey commented Feb 10, 2017

@HouzuoGuo let me give it a shot and Ill get back to you

@archey
Copy link
Collaborator

archey commented Feb 10, 2017

@HouzuoGuo all built and tests pass with 4.989s with the changes I made and the changes you requested. In my hash32.go file I still commented out the integer-smear function

@archey
Copy link
Collaborator

archey commented Feb 10, 2017

@HouzuoGuo heres the benchmark results if your curious

https://gist.github.com/archey/feb0d76e3c372a31eb01cba5fc883ddc

@HouzuoGuo
Copy link
Owner

Many thanks for your feedback, that looks quite good! I just put you into collaborators list, would you mind pushing the code to master please?

@archey
Copy link
Collaborator

archey commented Feb 11, 2017

I could make a pr, but Ill push to it if you'd like one moment

@archey
Copy link
Collaborator

archey commented Feb 11, 2017

@HouzuoGuo changes are pushed to master I removed the buildconstraint.go as it would of caused build issues for people.

@HouzuoGuo
Copy link
Owner

@archey could you please move the changed constants into hash32/64.go as well? Otherwise those changes will cause existing databases to malfunction.

@HouzuoGuo
Copy link
Owner

By the way, if you've also made changes to collection.go, please push it as well, and keep the modified constants also in the conditionally compiled .go files.

@archey
Copy link
Collaborator

archey commented Feb 11, 2017

Yeah let me get those issues fixed, Im still new to golang. I'll fix it up and see if the next push will be better.

@archey
Copy link
Collaborator

archey commented Feb 11, 2017

@HouzuoGuo the last push should be better I made a modified hashtable32 with the constants and a modified collection32 with the constants, they will only build on 386 and arm. Let me know if that was wrong, I can revert it and start over. It just made more sense to me that way.

@HouzuoGuo
Copy link
Owner

Oh I see you made the entire file collection.go and hashtable.go arch-dependent, this is OK too but then there is too much duplication between the arch dependent files. Let me fix it later today.

@archey
Copy link
Collaborator

archey commented Feb 12, 2017

@HouzuoGuo yeah my bad I can go back and fix it again, it just made more sense please fix whatever youd like

@HouzuoGuo
Copy link
Owner

No worries at all, I fixed it in master, please run tests in your 32-bit system and see if they still succeed. If so, this bug report may be finally closed.

@archey
Copy link
Collaborator

archey commented Feb 12, 2017 via email

@archey
Copy link
Collaborator

archey commented Feb 12, 2017

@HouzuoGuo builds fine and tests pass, with flying colors

@HouzuoGuo
Copy link
Owner

thanks very much, I'm glad to finally resolve this issue after 3 years 🧀

@archey
Copy link
Collaborator

archey commented Feb 12, 2017

@HouzuoGuo no problem I wish I had seen it sooner. I think spinning up a i686 vm would of worked as well

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

7 participants