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

TSM engine breaks windows compatibility #4358

Closed
corylanou opened this issue Oct 7, 2015 · 19 comments
Closed

TSM engine breaks windows compatibility #4358

corylanou opened this issue Oct 7, 2015 · 19 comments

Comments

@corylanou
Copy link
Contributor

$ GOOS=windows GOARCH=amd64 go build ./cmd/influxd
# github.com/influxdb/influxdb/tsdb/engine/tsm1
tsdb/engine/tsm1/tsm1.go:1799: undefined: syscall.Mmap
tsdb/engine/tsm1/tsm1.go:1799: undefined: syscall.PROT_READ
tsdb/engine/tsm1/tsm1.go:1799: undefined: syscall.MAP_SHARED
tsdb/engine/tsm1/tsm1.go:1836: undefined: syscall.Munmap

@pauldix @jwilder

@corylanou
Copy link
Contributor Author

@benbjohnson already handles this with boltdb and we should be able to do the same thing:

https://github.com/boltdb/bolt/blob/master/bolt_windows.go#L30

@otoolep
Copy link
Contributor

otoolep commented Oct 7, 2015

+1, sounds like a good idea.

@edokan
Copy link

edokan commented Oct 13, 2015

I've tried mmap-go package, and it seems to be working.

ref: HouzuoGuo/tiedot#7

@mvadu
Copy link
Contributor

mvadu commented Oct 23, 2015

Any ETA on the fix for this issue? Until its fixed, I have to stay in old builds!

@imrichardcole
Copy link

What is the latest version of influxdb that will compile on Windows without hitting this issue please?

@otoolep
Copy link
Contributor

otoolep commented Dec 2, 2015

@imrichardcole -- check out the 0.9.4 branch and compile that. It should work on Windows.

@imrichardcole
Copy link

@otoolep Ah many thanks, you'd actually responded before I had to chance to post back. I managed to get 0.9.4.2 to compile on Windows.

Can we expect ongoing Windows support or is it limited to 0.9.4.2?

@otoolep
Copy link
Contributor

otoolep commented Dec 2, 2015

@imrichardcole -- our Windows support is currently experimental, and the only guarantee we can provide is that when 1.0 comes out it will run on Windows.

@aadrian
Copy link

aadrian commented Dec 2, 2015

@otoolep the windows version is used mostly for development, not for production, so please release it even if it's experimental.

Thank you.

@corylanou
Copy link
Contributor Author

We are hoping to add experimental support back soon. This PR will hopefully allow for it if it gets merged for 0.9.6: #4788

Any help from the community will get this done sooner.

@mvadu
Copy link
Contributor

mvadu commented Dec 3, 2015

Until that time, I just built and published a release from 0.9.4 branch, https://github.com/mvadu/influxdb/releases/tag/v0.9.4.2

@mvadu
Copy link
Contributor

mvadu commented Dec 3, 2015

@corylanou @otoolep I tried adding mmap implementation for Windows based on MapViewOfFile. Used SliceHeader trick to change the pointer returned by MapViewOfFile to a byte slice. This will not call for any change in rest of tsm.

Submitted PR #4981

tsm1.go was directly calling syscall.Mmap not the custom defined mmap (defined both unix and windows versions).

mmap, err := syscall.Mmap(int(f.Fd()), 0, int(fInfo.Size()), syscall.PROT_READ, syscall.MAP_SHARED|MAP_POPULATE)

Hence removed Syscall.Mmap to use mmap, which should use respective plat form specific functions.

Note: This is the first time I am working on a fork repo. Please excuse me if I did some silly mistake. I am new to go, and was not able to test this PR with a Linux machine.

@rapport
Copy link

rapport commented Dec 8, 2015

Is this also related #4787 ?

@pauldix
Copy link
Member

pauldix commented Dec 8, 2015

I think this is now fixed in master. Can someone confirm? Bill Gates banned me from running Windows about a decade ago.

@rapport
Copy link

rapport commented Dec 8, 2015

It is not working for me but does appear to have less errors

ubuntu@NUC001:/.gvm/pkgsets/go1.5.2/global/src/github.com/influxdb/influxdb$ git pull
Already up-to-date.
ubuntu@NUC001:
/.gvm/pkgsets/go1.5.2/global/src/github.com/influxdb/influxdb$ go get -u -f ./...
ubuntu@NUC001:~/.gvm/pkgsets/go1.5.2/global/src/github.com/influxdb/influxdb$ GOOS=windows GOARCH=386 go build ./...
...
tsdb/engine/tsm1/mmap_windows.go:13: too many arguments to return

@fazalmajid
Copy link

@rapport no, #4787 is not related, tsm1 already broke Windows, but my first pull request #4788 made things worse. The second pull request #5027 fixes #4787 and doesn't introduce any additional regressions on Windows, but sadly it does nothing to fix #4358 either.

Apparently CircleCI (used by InfluxDB) has beta support for Windows, but it looks like an Enterprise (i.e. paid) feature: https://circleci.com/enterprise/azure

I wonder if the Microsoft Azure Open Source outreach team could be convinced to pick up the tab, but I have no idea how to get this done - my few contacts at Microsoft are at their Mobile division, not Azure.

@mvadu
Copy link
Contributor

mvadu commented Dec 8, 2015

@pauldix @fazalmajid master just has func mmap(f *os.File, offset int64, length int) ([]byte, error) { return nil, fmt.Errorf("mmap file not supported windows") so not really a working version.

I tried with #4981 and its work in progress now. I think I know the reason for the pending test failures, and should be able to fix in a day.

@mvadu
Copy link
Contributor

mvadu commented Dec 9, 2015

@pauldix I have fixed this issue temporarily with #5070, but it will not allow tsm1 to run in Windows yet. It is building fine, and runs okay with bz1 engine.

@fazalmajid
Copy link

As an alternative to CircleCI, Appveyor offers free Windows CI for open-source projects: http://www.appveyor.com/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants