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

Change S3Proxy from memory to filesystem storage #1665

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gaul
Copy link
Member

@gaul gaul commented May 23, 2021

This should avoid OutOfMemoryErrors seen with larger object sizes.
Enabled by fixes in S3Proxy 2.0.0. Also change LC_ALL to en_US.UTF-8
and add locales packages so that S3Proxy can create Unicode file
names. This commit requires Java 17 on macOS to allow creating
extended attributes for directories.

@gaul gaul requested a review from ggtakec May 23, 2021 12:27
@gaul
Copy link
Member Author

gaul commented May 23, 2021

It appears that S3Proxy filesystem backend doesn't handle Unicode paths properly:

[s3proxy] D 05-23 21:55:12.456 S3Proxy-Jetty-19 o.gaul.s3proxy.S3ProxyHandler:298 |::] request: Request(HEAD /s3fs-integration-test/testrun-12431/special%C2%B5)@10144307
...
[s3proxy] D 05-23 21:55:12.457 S3Proxy-Jetty-19 o.j.b.config.LocalBlobStore:56 |::] Retrieving blob with key testrun-12431/special? from container s3fs-integration-test
[s3proxy] D 05-23 21:55:12.457 S3Proxy-Jetty-19 o.j.b.config.LocalBlobStore:56 |::] Opening blob in container: s3fs-integration-test - testrun-12431/special?
[s3proxy] E 05-23 21:55:12.457 S3Proxy-Jetty-19 org.gaul.s3proxy.Main:238 |::] java.nio.file.InvalidPathException: Malformed input or input contains unmappable characters: /var/tmp/blobstore/s3fs-integration-test/testrun-12431/special?
[s3proxy] E 05-23 21:55:12.458 S3Proxy-Jetty-19 org.gaul.s3proxy.Main:238 |::]  at java.base/sun.nio.fs.UnixPath.encode(UnixPath.java:145)
[s3proxy] E 05-23 21:55:12.458 S3Proxy-Jetty-19 org.gaul.s3proxy.Main:238 |::]  at java.base/sun.nio.fs.UnixPath.<init>(UnixPath.java:69)
[s3proxy] E 05-23 21:55:12.458 S3Proxy-Jetty-19 org.gaul.s3proxy.Main:238 |::]  at java.base/sun.nio.fs.UnixFileSystem.getPath(UnixFileSystem.java:279)
[s3proxy] E 05-23 21:55:12.458 S3Proxy-Jetty-19 org.gaul.s3proxy.Main:238 |::]  at java.base/java.io.File.toPath(File.java:2329)
[s3proxy] E 05-23 21:55:12.458 S3Proxy-Jetty-19 org.gaul.s3proxy.Main:238 |::]  at org.jclouds.filesystem.strategy.internal.FilesystemStorageStrategyImpl.getBlob(FilesystemStorageStrategyImpl.java:399)

@gaul
Copy link
Member Author

gaul commented May 27, 2021

@CarstenGrohmann I tweaked your LC_ALL change to get S3Proxy/Java to interpret Unicode filenames correctly.

@gaul gaul force-pushed the s3proxy/filesystem branch 2 times, most recently from 895c927 to 5dd9bee Compare May 27, 2021 09:04
@gaul
Copy link
Member Author

gaul commented May 27, 2021

Something strange is going on -- I can get the CentOS 7 builder to pass but not the others. Also this works locally on my Fedora 34 desktop. So it seems Travis does something to the environment which confuses Java.

@ggtakec
Copy link
Member

ggtakec commented May 27, 2021

@gaul If the problem is Locale, Github Actions Runner doesn't seem to have en_US.UTF-8. (It seems that there are only C, C.UTF-8, POSIX)
It looks like you have to set the locale to Runner. But the localdef command also gives an error because of without a charactor map, so you may have to install a package such as locales.

@gaul gaul force-pushed the s3proxy/filesystem branch 2 times, most recently from 8565930 to b1eb15d Compare May 28, 2021 00:01
@gaul
Copy link
Member Author

gaul commented May 28, 2021

It looks like you have to set the locale to Runner. But the localdef command also gives an error because of without a charactor map, so you may have to install a package such as locales.

Thanks for the hint! Now all the Linux builders succeed but macOS fails:

mkdir: s3fs-integration-test/testrun-23762: Invalid argument

This may be a problem with S3Proxy; I will investigate.

@gaul gaul force-pushed the s3proxy/filesystem branch 2 times, most recently from 9896b56 to 2595074 Compare May 28, 2021 03:21
@gaul
Copy link
Member Author

gaul commented May 28, 2021

I understand the macOS failures -- S3Proxy uses extended attributes to signify directories. Java on macOS lacks support for xattr before Java 17: https://bugs.openjdk.java.net/browse/JDK-8030048. We can upgrade the macOS builder when Homebrew upgrades to JDK 17 (probably in September).

Future note to self: there are some InaccessibleObjectException which may require some upgrading Guice and thus jclouds and thus S3Proxy...

gaul added a commit to gaul/s3fs-fuse that referenced this pull request Jun 20, 2021
This allows tests to pass in a Debian VM on Chromebook.
References s3fs-fuse#1665.
gaul added a commit to gaul/s3fs-fuse that referenced this pull request Jun 20, 2021
This allows tests to pass using the S3Proxy filesystem provider.
References s3fs-fuse#1665.
ggtakec pushed a commit that referenced this pull request Jun 20, 2021
This allows tests to pass using the S3Proxy filesystem provider.
References #1665.
@gaul gaul force-pushed the s3proxy/filesystem branch 2 times, most recently from f9dc6b4 to 56ed505 Compare September 19, 2021 23:03
@gaul
Copy link
Member Author

gaul commented Sep 19, 2021

Homebrew recently updated to openjdk 17:

https://formulae.brew.sh/formula/openjdk#default

But it doesn't seem like this has propagated yet since Travis fails.

@gaul
Copy link
Member Author

gaul commented Sep 21, 2021

Seems like the GitHub-supported way to due this is currently blocked on actions/runner-images#4085.

@gaul
Copy link
Member Author

gaul commented Jan 4, 2022

Also blocked by gaul/s3proxy#396.

@gaul gaul marked this pull request as draft January 8, 2022 11:36
@gaul gaul force-pushed the s3proxy/filesystem branch 2 times, most recently from 4c49eff to 3672139 Compare April 3, 2022 11:50
@gaul
Copy link
Member Author

gaul commented Apr 3, 2022

Something strange here -- brew reports:

Warning: openjdk 17.0.2 is already installed and up-to-date.

but java -version reports:

openjdk version "1.8.0_322"

@gaul gaul force-pushed the s3proxy/filesystem branch 7 times, most recently from 2c3aee9 to 5065ca2 Compare April 7, 2022 13:47
@gaul
Copy link
Member Author

gaul commented Apr 7, 2022

I tried this suggestion but it didn't work:

For the system Java wrappers to find this JDK, symlink it with
  sudo ln -sfn /usr/local/opt/openjdk@17/libexec/openjdk.jdk /Library/Java/JavaVirtualMachines/openjdk-17.jdk

This is a little complicated for me to debug since my MacBook 2010 cannot use the latest Homebrew. Maybe #1851 is easier?

@ggtakec
Copy link
Member

ggtakec commented Apr 23, 2022

@gaul
This problem seems to be solved by setting PATH.
When I tried it, the test was successful by modifying ci.yml as follows:

(L:144)   PATH=/usr/local/opt/openjdk@17/bin:${PATH} java -version # shows version 17 as expected
(L:175)   PATH=/usr/local/opt/openjdk@17/bin:${PATH} make ALL_TESTS=1 check -C test || (test/filter-suite-log.sh test/test-suite.log; exit 1)

@gaul gaul force-pushed the s3proxy/filesystem branch 3 times, most recently from e4927ff to f8ea4b8 Compare August 4, 2023 12:17
This should avoid OutOfMemoryErrors seen with larger object sizes.
Enabled by fixes in S3Proxy 2.0.0.  Also change LC_ALL to en_US.UTF-8
and add locales packages so that S3Proxy can create Unicode file
names.  This commit requires Java 17 on macOS to allow creating
extended attributes for directories.
@gaul
Copy link
Member Author

gaul commented Aug 5, 2023

Some tests fail due to overfull /var/tmp. Maybe https://github.com/gaul/s3proxy/wiki/Middleware-large-object-mocking is better than filesystem?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

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.

None yet

2 participants