*: build phony etcd server binary for unsupported architectures #4721
Conversation
| @@ -12,6 +12,8 @@ | |||
| // See the License for the specific language governing permissions and | |||
| // limitations under the License. | |||
|
|
|||
| // +build amd64 | |||
heyitsanthony
force-pushed
the
build-scary-archs
branch
2 times, most recently
from
b4b88ad to
89774e4
Compare
|
update https://github.com/coreos/etcd/blob/master/README.md#32-bit-systems to reflect that etcd will build but simply print out waring instead of starting? |
|
Hey, please please check out #4715 first |
| @@ -30,5 +30,5 @@ import ( | |||
| // silently ignore this flag. Please update your kernel to prevent this. | |||
| var boltOpenOptions = &bolt.Options{ | |||
| MmapFlags: syscall.MAP_POPULATE, | |||
| InitialMmapSize: InitialMmapSize, | |||
| InitialMmapSize: int(InitialMmapSize), | |||
There was a problem hiding this comment.
This will make etcd unreliable for amd64, and was the cause to the #4715 (comment) failure. If you merge this etcd won't work. See https://play.golang.org/p/3Slqmtf9JC
There was a problem hiding this comment.
No, that was not the reason for the test failure. I tested it with a 256MB mmap reservation and it still failed.
Likewise, go playground compiles with arch "amd64p32", which is why the ints are getting truncated. I tested on an amd64 arch and it gives the correct 64-bit value.
There was a problem hiding this comment.
Why are you changing this anyway? AFAIU it worked fine on amd64 before.
There was a problem hiding this comment.
bolt.Options expects an int; declaring InitialMmapSize as int64 so that arm builds won't complain causes a type error without the int cast
We don't qualify etcdserver for anything other than amd64, so don't build binaries that are untested and might be unreliable.
heyitsanthony
force-pushed
the
build-scary-archs
branch
from
89774e4 to
5f304b4
Compare
|
@heyitsanthony #4715 doesn't break |
|
@luxas OK, I see it's fine now. However, it's still emitting an unstable server binary for arm. Have you tried running the tests on an arm machine? I'm very uncomfortable putting that in mainline for a system that's meant to be reliable. |
|
LGTM. IMHO this is a better solution to #4715. It fix the build while preventing unexpected use of 32 bit systems. Let's get this in as fast as possible. |
|
readme updated /cc @xiang90 |
*: build phony etcd server binary for unsupported architectures
|
Why aren't you writing out a really big warning that the binary isn't supported only? That's the way to go. Then you aren't shutting doors for the community. We all know you're not supporting it commercially. There are folks that use open source just for fun and hack on cheap ARM devices just because they can. |
|
@luxas Here's why: people ignore docs; a big warning won't do much since people already ignore the 32-bit support warning. Right now arm support is clearly broken even when the source is hacked to build it. I worry people will build broken systems with etcd with the mistaken impression that since it runs it must be fine (this actually happens). I believe this is a legitimate concern and somewhat reckless to disregard. If you want an arm build of etcdserver, it's a two line change (maybe three?). Likewise, the client still builds fine; etcdserver can be hosted on amd64 nodes. Fully supporting etcd on arm would be great but it needs to be stable first; anything less seems rather irresponsible. |
We don't qualify etcdserver for anything other than amd64, so don't
build binaries that are untested and might be unreliable.