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

Mode 1448 - Store binary files in a relational database #440

Merged
merged 10 commits into from Jul 26, 2012

Conversation

okulikov
Copy link
Member

I didn't add any kind of unit test because dependecy from local database most probably will cause build failures.

@okulikov
Copy link
Member Author

-Separated content into chunks
-Separated metadata and payload
-Added transaction manager to maintain two separate caches
-Added configuration for datasource via JNDI

-Added expire time (it should reduce memory usage by offloading from memory idle content)

Disatventage is that it still affected by large content:

  • out of memory if large file(s) will be stored in a cache. as quick fix we can configure small idle time
  • "stop the world" problem from GC side. Chunk objects are immutable (usage of mutable object with infinispan is dangenrous) and all will be collected.

The most safe way is to implement this store using plain JDBC and mutable chunks. Metadata can be cached.

The most safe solution for large content will be

@okulikov
Copy link
Member Author

Plain JDBC version ready

*/
private static String blob(Connection connection, int size) throws BinaryStoreException {
try {
String name = connection.getMetaData().getDatabaseProductName();
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this method is not called too much (only during initialization), but might want to lowercase the product name to prevent repeatedly calling toLowerCase().

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@rhauch
Copy link
Contributor

rhauch commented Jul 18, 2012

Overall, I really like this new approach. Very nice work, Oleg. As we discussed, this proof of concept is probably ready to be polished, including your suggestion of moving many of the utility methods and potentially customizable methods into a Database class (which can be subclassed/specialized for specific DBMSes if needed).

Great work!

@okulikov
Copy link
Member Author

I think I found another minor problem with content storage in latest impl. Caused by key, it is unique for each invocation irrespective of content and thus same content can be stored more then once. I assume we need to compute hash after content store into intermediate table and check the for existance before "actual" insert.

@rhauch
Copy link
Contributor

rhauch commented Jul 20, 2012

I think I understand the issue: we don't know the SHA-1 of the content until we've read it completely, and until we read it we can't know whether the value has been stored. I can think of several approaches:

  1. Always write directly to the database, but design it such that duplicates can be tolerated and cleaned up. This doesn't save any time storing a binary value, but it does help to reduce our storage costs as duplicate binary values would eventually be cleaned up. This might be a bit more complicated of a solution, though.
  2. Write to a temporary file first (computing the SHA-1 as we go), and then check the database for the SHA-1 and write the temp file into the database BLOB if needed. With this approach, we can use the SHA-1 as the primary key. Note that this insert is kind of a double-checked lock, because even though we are attempting to insert only after checking that the SHA-1 wasn't in the database, another transaction (from another process in the cluster) might commit between our check and our insert.

The FileSystemBinaryStore writes to a temporary file, and once it knows the SHA-1 looks for an existing file. If a binary value doesn't exist for the SHA-1, it moves the temporary file to it's storage area (and thus doesn't have to write it twice).

The interesting thing is that the likelihood of an application storing the same file multiple times varies, and in most cases is probably pretty low. On one hand, that means that most of the time it might be okay to always insert the value into the database (option 1 above) since it will not likely clash. On the other, we do have to deal with clashes, and such a design might be more complicated. (For example, we can't make a unique constraint on the SHA-1 column if we want to put NULL into that column, so we'd have to play some tricks with the schema or accept non-unique constraints. Plus, we'd want the schema to be such that the cleanup command is pretty simple; e.g., DELETE FROM content_store WHERE sha1key IS NULL or DELETE FROM content_store WHERE used IS 0.)

BTW, our SecureHash.createHashingStream(...) utility method will wrap an InputStream so that after it is finished we can simply ask it for the SHA-1 of the (already read) content. Both approaches listed above should use this so that we don't have to stream thru the contents more than once.

Does that help at all?

@rhauch rhauch closed this Jul 20, 2012
@rhauch rhauch reopened this Jul 20, 2012
@rhauch
Copy link
Contributor

rhauch commented Jul 20, 2012

(Closed accidentally, so had to reopen. Sorry.)

Thinking aloud ...

I just thought of a slight variation to the second approach in my previous comment. What if the DatabaseBinaryStore contained a FileSystemBinaryStore as a local cache, and wrote the binary to that cache first. The FileSystemBinary store would actually compute the key (and SHA-1)? And so that the FileSystemBinaryStore doesn't permanently store the file (we'd want it to be a cache, not a full copy of the values), the DatabaseBinaryStore might immediately mark the value in the FileSystemBinaryStore as unused (meaning the FileSystemBinaryStore would make it available for cleanup.) Plus, using it as a cache might mean that recently-read BinaryValues are locally available, saving trips to the database.

This might not even be as easy has just having the DatabaseBinaryStore write out to a temporary file (kind of like the FileSystemBinaryStore does).

@okulikov
Copy link
Member Author

Talking about likelihood to store duplicate values I have concern about
accidential inserts. Like double press "submit" button, or other
application's mistakes. The next concern is how to find duplicate content
(if it happen) using standard DB utilities and then find out which is
"broken". I am sure it is not too difficult for us to knock down this
potential problem :)

So... My vote is for approach with FileSystemBinaryStore as local cache.
Since we are talking about realy large objects the savings on data trip are
really important.

2012/7/20 Randall Hauch <
reply@reply.github.com

(Closed accidentally, so had to reopen. Sorry.)

I just thought of a slight variation to the first approach in my previous
comment. What if the DatabaseBinaryStore contained a FileSystemBinaryStore
as a local cache, and wrote the binary to that cache first. The
FileSystemBinary store would actually compute the key (and SHA-1). And so
that the FileSystemBinaryStore doesn't permanently store the file (we'd
want it to be a cache, not a full copy of the values), the
DatabaseBinaryStore might immediately mark the value in the
FileSystemBinaryStore as unused (meaning the FileSystemBinaryStore would
make it available for cleanup.) Plus, using it as a cache might mean that
recently-read BinaryValues are locally available, saving trips to the
database.

I'm not sure whether this would be easier or harder than just having the
DatabaseBinaryStore write out to a temporary directory.


Reply to this email directly or view it on GitHub:
#440 (comment)

@rhauch rhauch merged commit 0f0627b into ModeShape:master Jul 26, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants