Skip to content

Commit

Permalink
bump deps to 64bit os
Browse files Browse the repository at this point in the history
  • Loading branch information
Bernhard Posselt committed Apr 27, 2016
1 parent 2234ef5 commit 4f5e4df
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,13 @@ For further developer and user documentation please visit [the wiki](https://git
are listed on the [ownCloud apps overview](https://github.com/owncloud/core/wiki/Apps)

## Dependencies
* At least a 64bit operating system, otherwise sync will not work

This comment has been minimized.

Sign in to view

Copy link
@paroj

paroj Jul 23, 2016

is this also true for the current API or just for v2?

This comment has been minimized.

Sign in to view

Copy link
@BernhardPosselt

BernhardPosselt Jul 23, 2016

Member

True for v2 and depends on your setup. For installations with more than 500 users you will run into issues where ids are running out.

This comment has been minimized.

Sign in to view

Copy link
@BernhardPosselt

BernhardPosselt Jul 23, 2016

Member

All in all this will be super hard required in a few weeks, so not requiring 64bit atm makes no difference imo

This comment has been minimized.

Sign in to view

Copy link
@paroj

paroj Jul 23, 2016

thats quite unfortunate for all the ARMv7 machines out there. would a string id be a viable alternative?

This comment has been minimized.

Sign in to view

Copy link
@BernhardPosselt

BernhardPosselt Jul 23, 2016

Member

Well, sorting on strings does not work like expected if they dont have the same length and it is incredibly slow. Since the "did something change" check should be fast I dont see this as a viable alternative.

Also I'd recommend to upgrade to a newer Raspberry pie like the model 3 (if that's what you are using) or even better use a vserver since the Raspberry pie is terrible in terms of IO (which is basically what the News app does)

This comment has been minimized.

Sign in to view

Copy link
@paroj

paroj Jul 23, 2016

I am still quite dazed that PHP does not have a long data type. Anyway.. requiring a 64bit CPU because of some ids seems over the top to me.

  • What about using doubles? If you only store integers they are exact (comparable with ==) up to 52bits.
  • Or limiting the amount of all items to INT_MAX and wrapping around the id once it is reached?

This comment has been minimized.

Sign in to view

Copy link
@BernhardPosselt

BernhardPosselt Jul 23, 2016 •
edited
Loading

Member

What about using doubles? If you only store integers they are exact (comparable with ==) up to 52bits.

Autoincrement will not work for that, I also fear that we will run into uniqueness issues and doubles as primary keys strikes me as a hack

Or limiting the amount of all items to INT_MAX and wrapping around the id once it is reached?

I wouldnt want to implement that even if it worked. Currently ids are used for ordering items (it's not like we've tried something else, it's complicated) so that will screw ordering and requiring logic for autoincrement primary keys is fucked up :)

TL;DR: use a 64bit CPU, it's 2016, no need to dwell in the past

This comment has been minimized.

Sign in to view

Copy link
@paroj

paroj Jul 23, 2016 •
edited
Loading

Autoincrement will not work for that, I also fear that we will run into uniqueness issues and doubles as primary keys strikes me as a hack

I think you can use BIGINT on the DB side to get autoincrement which the DB driver will then convert to doubles when passing them to PHP:
https://neunzehn83.de/blog/2010/09/14/mysqls-bigint-und-php.html

TL;DR: use a 64bit CPU, it's 2016, no need to dwell in the past

its just that I like to use stuff until it genuinely breaks.

This comment has been minimized.

Sign in to view

Copy link
@p5n

p5n Jul 23, 2016

use a 64bit CPU, it's 2016, no need to dwell in the past

It is true for x86 but is not true for mips and arm at least.

See for example https://archlinuxarm.org/platforms

This comment has been minimized.

Sign in to view

Copy link
@BernhardPosselt

BernhardPosselt Jul 23, 2016 •
edited
Loading

Member

Sure however 64bit ARMv8 is already available and cheap as well. Apart from that the News app is neither optimized nor intended to run on the MIPS or armv7

This comment has been minimized.

Sign in to view

Copy link
@LennartRoeder

LennartRoeder Jul 24, 2016 •
edited
Loading

As far as I understand, the issue is, that 32 bit can't handle over 500 Users. Well nobody is gonna do that on ARM hardware anyway ... I hope.
How about a check, that uses the 64 bit IDs if the CPU is capable of it and stays with 32 if not?
I am not sure how much duplicate code this will produce, but it would be the best of the 2 worlds.

Telling all the people on arm to buy new hardware is really a bad idea. I for my part would rather keep an outdated version, or spin up something like Tiny Tiny RSS than throw out my Odroid XU. It's perfectly working with Ubuntu 16.04, has a quad core CPU, 2GB Ram with an SSD over USB 3.0.

edit:
Another argument against enforcing 64 bit:
For such big installations news will not be the primary usecase. They will want files and maybe calendar or contacts.
Our department at the university has around 5K clients in their OC installation and they don't bother with apps at all. All you get is gallery and files.

This comment has been minimized.

Sign in to view

Copy link
@robje

robje Jul 27, 2016

@BernhardPosselt wrote

... neither optimized nor intended to run on the MIPS or armv7

Isn't PHP supposed to abstract that away?

I, like @thinking-aloud run on a small ARM device (QNAP NAS with Debian).
The device serves the needs I have and I'm not intending to replace the NAS with a more power hungry device.

My guess is there are more people out there with similar installations, especially since there are plenty of NAS installation instructions out there.
So requiring 64-bit excludes a lot of users IMHO.

This comment has been minimized.

Sign in to view

Copy link
@mcorteel

mcorteel Jul 28, 2016

My shared hosting provider does not provide a 64 bit option, so I would have to change just for the news app... That's a shame.
And I don't think that

use a 64bit CPU, it's 2016, no need to dwell in the past

is an appropriate answer. This requirement is rather rare and I think that it excludes a lot of users.

Anyway, I had a good run with the news app, thanks for your work!

This comment has been minimized.

Sign in to view

Copy link
@BernhardPosselt

BernhardPosselt Jul 31, 2016

Member

Only other solution I see here is to go for native DateTime types which effectively means dropping support for sqlite. Apart from that I'd need to investigate if nextcloud supports dateime types.

I'm sure there will be lots of negative feedback for dropping sqlite however ;D

This comment has been minimized.

Sign in to view

Copy link
@tallie-andrew

tallie-andrew Aug 16, 2016

I second this. I run owncloud because I want to have my own cloud and not be dependent on some malicious commercial provider. I don't have hundreds of users, just one, and I run on a low-power plug computer (Arm v5TE) with sqlite. I would very strongly suspect that anyone running owncloud on 32 bit hardware is running a small installation and will never hit the 32 bit integer limit. People running gigantic >500 user installations are already on 64-bit hardware.
* PHP >= 5.6
* ownCloud >= 9.0
* libxml >= 2.7.8 (2.9 recommended)
* php-curl
* iconv
* SimpleXML
* PHP >= 5.6

## Build Dependencies
These Dependencies are only relevant if you want to build the source code:
Expand Down

11 comments on commit 4f5e4df

@Loki3000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't use GUID as an identifier? It's unique, it's 128bit, it's not require 64bit system and at last it's allowed to synchronize different devices/databases without rebuilding identifiers.

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guid is not unique and this is about change detection (think of marking an item read on the server) so this is the wrong place to look at ;)

@Loki3000
Copy link
Contributor

@Loki3000 Loki3000 commented on 4f5e4df Aug 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guid is not unique

Ok. It's "almost" unique:)
I'm not understand how unique identifier links to change detection?

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Loki3000 right, because it doesnt :)

The 64bit requirement is about last modified timestamps in miliseconds for HTTP caching. Think of it like "give me all changes that are newer than this timestamp"

@Loki3000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's better to use BC Math except 64bit?
http://php.net/manual/en/ref.bc.php

Could you show in which place you use bigint as a number in php? Maybe it's possible to move such comparison completely to database engine?

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparison is in php and database, basically: select all entries with a timestamp lower than the current one and don't send all properties to the client if the given timestamp is lower than the current one. I don't think you can use a big number lib for that

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically once you construct the number it's game over on 32 bit and the number is passed as etag from the client (as string) but needs to be converted to an integer for pdo

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is unless it's possible to pass strings for bigints (if so we can work around that)

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparison in php can fully be done using datetime objects btw

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tldr: If the pdo string for bigints works we can drop the 64 bit restriction

@BernhardPosselt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion continues here: #9

@Loki3000 thanks for the input, you brought me on the right track ;)

Need one last confirmation, please check out #9 (comment)

Please sign in to comment.