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

update to enable usage on windows #12

Merged
merged 9 commits into from
Jan 8, 2019
Merged

Conversation

rfecher
Copy link
Contributor

@rfecher rfecher commented Dec 5, 2018

based on the other comment in this file referencing, it is known that this won't work in windows, and results in fatal exceptions https://grokbase.com/t/lucene/dev/1519kz2s50/recent-java-9-commit-e5b66323ae45-breaks-fsync-on-directory

It seems appropriate to allow for the situation where a file channel cannot be created on a directory because in that situation (Windows) fsync is not applicable either.

src/main/java/com/oath/halodb/DBDirectory.java Outdated Show resolved Hide resolved
src/main/java/com/oath/halodb/DBDirectory.java Outdated Show resolved Hide resolved
src/main/java/com/oath/halodb/DBDirectory.java Outdated Show resolved Hide resolved
src/main/java/com/oath/halodb/DBDirectory.java Outdated Show resolved Hide resolved
@rfecher
Copy link
Contributor Author

rfecher commented Jan 2, 2019

@amannaly the requested changes have been made

@amannaly
Copy link
Collaborator

amannaly commented Jan 4, 2019

@rfecher thank you for the changes. Looks like this change has surfaced a bug in close() call due to which the travis build is failing. I have added a fix for the bug.

Could you please add the fix also to your PR so that the build can pass.

Also Yahoo requires all external contributors to sign agree to this document. Could you please take a look and sign?
Basically, you are agreeing to contribute your changes to this open source project.

@rfecher
Copy link
Contributor Author

rfecher commented Jan 7, 2019

@amannaly I pulled in your fix. I also signed the CLA. Thanks!

@amannaly amannaly merged commit 52dfc19 into yahoo:master Jan 8, 2019
@amannaly
Copy link
Collaborator

amannaly commented Jan 8, 2019

@rfecher Merged. Thank you.

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

Successfully merging this pull request may close these issues.

2 participants