[xwiki-devs] [PROPOSAL] Add XWikiAttachmentContent#setContent() which returns an OutputStream to write content to.
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too. I would like to add to XWikiAttachmentContent: public OutputStream addContent(); which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field. WDYT? Caleb
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ? Thanks, Marius On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote:
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too.
I would like to add to XWikiAttachmentContent:
public OutputStream addContent();
which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
+1 for the idea in general but same comment than Marius On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote:
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ?
Thanks, Marius
On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote:
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too.
I would like to add to XWikiAttachmentContent:
public OutputStream addContent();
which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember. If you guys like getOutputStream(), I'm happy enough with it. Caleb On 08/07/2012 03:24 AM, Thomas Mortagne wrote:
+1 for the idea in general but same comment than Marius
On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote:
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ?
Thanks, Marius
On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote:
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too.
I would like to add to XWikiAttachmentContent:
public OutputStream addContent();
which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream() +0 for getOutputStream() -0 for other previous proposals. On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember.
If you guys like getOutputStream(), I'm happy enough with it.
Caleb
On 08/07/2012 03:24 AM, Thomas Mortagne wrote:
+1 for the idea in general but same comment than Marius
On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote:
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ?
Thanks, Marius
On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote:
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too.
I would like to add to XWikiAttachmentContent:
public OutputStream addContent();
which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Denis Gervalle SOFTEC sa - CEO eGuilde sarl - CTO
On 08/07/2012 03:43 PM, Denis Gervalle wrote:
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream()
+1 as well.
+0 for getOutputStream() -0 for other previous proposals.
On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember.
If you guys like getOutputStream(), I'm happy enough with it.
Caleb
On 08/07/2012 03:24 AM, Thomas Mortagne wrote:
+1 for the idea in general but same comment than Marius
On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote:
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ?
Thanks, Marius
On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote:
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too.
I would like to add to XWikiAttachmentContent:
public OutputStream addContent();
which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field.
WDYT?
Caleb
-- Sergiu Dumitriu http://purl.org/net/sergiu/
On 08/07/2012 03:44 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:43 PM, Denis Gervalle wrote:
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream()
+1 as well.
Actually, one thing I don't like is that the API is getting even bigger, with lots of different ways of doing the same thing. Ideally, there should be only one way of providing the content, and only one way of retrieving it. And IMO working with byte streams is the right way, and I think that the current setContent(InputStream) method is the one that's best. Returning an OutputStream where the caller can write the content is opening the door to lots of potential problems. Providing such a method becomes API, and APIs should be well designed. And an OutputStream in my head means some implicit constraints that must be well documented: - should the stream be closed at the end? - must the response be written before a transaction ends? - what if nothing is written in the stream, does that mean that the old content is preserved, or that the attachment is going to be truncated? Basically, this is a _push_ API, and I for one prefer _pull_ APIs, since the backend will get the data when it needs it, it doesn't have to document how long it's going to wait for something to be pushed. I agree that our current way of dealing with Hibernate is not right. We should use proper blob streams for big data, like attachment content. And Hibernate's LobCreator expects a Blob object that offers access to its content via an InputStream that can be read. Again, Hibernate expects an input stream from which it will read when it needs to, it doesn't allow you to write the content into an OutputStream when you want. So, consider this a -1 until you can convince me that it's indeed something we want to include in our APIs.
+0 for getOutputStream() -0 for other previous proposals.
On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember.
If you guys like getOutputStream(), I'm happy enough with it.
Caleb
On 08/07/2012 03:24 AM, Thomas Mortagne wrote:
+1 for the idea in general but same comment than Marius
On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote:
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ?
Thanks, Marius
On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote:
Hi, In the development of Cassandra attachments, I found I want to load an attachment one chunk at a time and write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too.
I would like to add to XWikiAttachmentContent:
public OutputStream addContent();
which returns an OutputStream that allows writing the attachment content and upon close, sets the attachment content as dirty and resets the size field.
WDYT?
Caleb
-- Sergiu Dumitriu http://purl.org/net/sergiu/
On 08/07/2012 10:52 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:44 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:43 PM, Denis Gervalle wrote:
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream()
+1 as well.
Actually, one thing I don't like is that the API is getting even bigger, with lots of different ways of doing the same thing. Ideally, there should be only one way of providing the content, and only one way of retrieving it. And IMO working with byte streams is the right way, and I think that the current setContent(InputStream) method is the one that's best.
Returning an OutputStream where the caller can write the content is opening the door to lots of potential problems. Providing such a method becomes API, and APIs should be well designed. And an OutputStream in my head means some implicit constraints that must be well documented:
I can answer for the implementation which I wrote as a result of this discussion.
- should the stream be closed at the end?
Yes, if it is not closed then the content does not become "live".
- must the response be written before a transaction ends?
It doesn't matter when it is written since it's just dealing with FileItems.
- what if nothing is written in the stream, does that mean that the old content is preserved, or that the attachment is going to be truncated?
The attachment becomes is made empty. It is legal to have a 0 byte file.
Basically, this is a _push_ API, and I for one prefer _pull_ APIs, since the backend will get the data when it needs it, it doesn't have to document how long it's going to wait for something to be pushed.
I agree that our current way of dealing with Hibernate is not right. We should use proper blob streams for big data, like attachment content. And Hibernate's LobCreator expects a Blob object that offers access to its content via an InputStream that can be read. Again, Hibernate expects an input stream from which it will read when it needs to, it doesn't allow you to write the content into an OutputStream when you want.
There are a few issues here. When I investigated this, Hibernate's blob handling was (still is?) just a wrapper around blob management in the database which basically doesn't work. Oracle has a proprietary blob storage method which works with streams, I think Hibernate does not support this though. As I remember, mysql and postgres have blob APIs but they buffer everything in ram making it nonsensical. This was investigated to solve attachment memory consumption issues and it was concluded that it does not work, leading to FS attachment store. A solution which will work is to piece out the attachment and store each piece separately, flushing the PersistenceManager or Session between save/load operations. they can be run in discrete transactions as long as the metadata contains a pointer to the pieces and the old pieces are not removed until after all of the new pieces are saved. On the load, you are loading a list of attachment pieces one by one and their content must be written out before it is removed from memory. The options for making this work are: * copy the content into a temp file then copy it again the InputStream from that file. This double buffers the content. * use PipedOutputStream to bounce it between two threads. (which I am doing now but need to debug a problem with it.) * extend XWikiAttachmentContent for this use case. * add a method to XWikiAttachmentContent which allows for writing to it using an OutputStream. Since this pattern would be applicable to Hibernate and the technology which came from Cassandra attachments could be ported into the main trunk, I wanted to do the latter. If you are not swayed then I will return to evaluating the former methods. Caleb
So, consider this a -1 until you can convince me that it's indeed something we want to include in our APIs.
+0 for getOutputStream() -0 for other previous proposals.
On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember.
If you guys like getOutputStream(), I'm happy enough with it.
Caleb
On 08/07/2012 03:24 AM, Thomas Mortagne wrote:
+1 for the idea in general but same comment than Marius
On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote:
I understand the need and I'm +1 but I don't like the method name (neither setContent() nor addContent()). WDYT about getOutputStream() ?
Thanks, Marius
On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle <[email protected]> wrote: > Hi, > In the development of Cassandra attachments, I found I want to > load an attachment one chunk at a time and > write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too. > > I would like to add to XWikiAttachmentContent: > > public OutputStream addContent(); > > which returns an OutputStream that allows writing the attachment content and upon close, > sets the attachment content as dirty and resets the size field. > > WDYT? > > Caleb
My changes are in a the feature-attachment-outputstream branch https://github.com/xwiki/xwiki-platform/commits/feature-attachment-outputstr... IMO the Hibernate blob handling is never really going to work in a cross-platform way. Even if that is not true, it absolutely won't work with Cassandra. For loading an attachment piecewise, OutputStreams are not the only possible solution but they are (in my analysis) the best. If we hold off on adding the API because there is no consumer in platform proper, I'll have to code around it and find another (inferior) solution and if piecewise attachment loading ever does reach platform proper, everything will be developed and debugged around inferior solution. That's my case for it, let me know if you have any specific concerns or if your veto is on the general principle. I want to get an idea of whether this should be cleaned up or killed quickly. Thanks, Caleb On 08/07/2012 11:31 PM, Caleb James DeLisle wrote:
On 08/07/2012 10:52 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:44 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:43 PM, Denis Gervalle wrote:
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream()
+1 as well.
Actually, one thing I don't like is that the API is getting even bigger, with lots of different ways of doing the same thing. Ideally, there should be only one way of providing the content, and only one way of retrieving it. And IMO working with byte streams is the right way, and I think that the current setContent(InputStream) method is the one that's best.
Returning an OutputStream where the caller can write the content is opening the door to lots of potential problems. Providing such a method becomes API, and APIs should be well designed. And an OutputStream in my head means some implicit constraints that must be well documented:
I can answer for the implementation which I wrote as a result of this discussion.
- should the stream be closed at the end?
Yes, if it is not closed then the content does not become "live".
- must the response be written before a transaction ends?
It doesn't matter when it is written since it's just dealing with FileItems.
- what if nothing is written in the stream, does that mean that the old content is preserved, or that the attachment is going to be truncated?
The attachment becomes is made empty. It is legal to have a 0 byte file.
Basically, this is a _push_ API, and I for one prefer _pull_ APIs, since the backend will get the data when it needs it, it doesn't have to document how long it's going to wait for something to be pushed.
I agree that our current way of dealing with Hibernate is not right. We should use proper blob streams for big data, like attachment content. And Hibernate's LobCreator expects a Blob object that offers access to its content via an InputStream that can be read. Again, Hibernate expects an input stream from which it will read when it needs to, it doesn't allow you to write the content into an OutputStream when you want.
There are a few issues here. When I investigated this, Hibernate's blob handling was (still is?) just a wrapper around blob management in the database which basically doesn't work. Oracle has a proprietary blob storage method which works with streams, I think Hibernate does not support this though. As I remember, mysql and postgres have blob APIs but they buffer everything in ram making it nonsensical. This was investigated to solve attachment memory consumption issues and it was concluded that it does not work, leading to FS attachment store.
A solution which will work is to piece out the attachment and store each piece separately, flushing the PersistenceManager or Session between save/load operations. they can be run in discrete transactions as long as the metadata contains a pointer to the pieces and the old pieces are not removed until after all of the new pieces are saved. On the load, you are loading a list of attachment pieces one by one and their content must be written out before it is removed from memory.
The options for making this work are:
* copy the content into a temp file then copy it again the InputStream from that file. This double buffers the content. * use PipedOutputStream to bounce it between two threads. (which I am doing now but need to debug a problem with it.) * extend XWikiAttachmentContent for this use case. * add a method to XWikiAttachmentContent which allows for writing to it using an OutputStream.
Since this pattern would be applicable to Hibernate and the technology which came from Cassandra attachments could be ported into the main trunk, I wanted to do the latter. If you are not swayed then I will return to evaluating the former methods.
Caleb
So, consider this a -1 until you can convince me that it's indeed something we want to include in our APIs.
+0 for getOutputStream() -0 for other previous proposals.
On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember.
If you guys like getOutputStream(), I'm happy enough with it.
Caleb
On 08/07/2012 03:24 AM, Thomas Mortagne wrote:
+1 for the idea in general but same comment than Marius
On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea <[email protected]> wrote: > I understand the need and I'm +1 but I don't like the method name > (neither setContent() nor addContent()). WDYT about getOutputStream() > ? > > Thanks, > Marius > > On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle > <[email protected]> wrote: >> Hi, >> In the development of Cassandra attachments, I found I want to >> load an attachment one chunk at a time and >> write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too. >> >> I would like to add to XWikiAttachmentContent: >> >> public OutputStream addContent(); >> >> which returns an OutputStream that allows writing the attachment content and upon close, >> sets the attachment content as dirty and resets the size field. >> >> WDYT? >> >> Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
Le 9 août 2012 à 10:28, Caleb James DeLisle a écrit :
For loading an attachment piecewise, OutputStreams are not the only possible solution but they are (in my analysis) the best.
I absolutely agree to this. I find the usage of byte arrays should be even deprecated (still doable while staying in 4.x?). I've been bragging about it from time to time... but could never contribute to it indeed. HttpClient does complain everytime you try to allocate entities in byte-arrays. Maybe that's too explicit but that's effective to force the developer into finding alternate solutions. I've seen things slightly more explicit than outputstream and inputstream floating around, things related to FileUpload if I remember well. In particular, I think it is best practice to carry the length as far away into the API as possible thus allowing the best buffering and storage strategy. Paul
In case you're interested in the design, here is a very rough (not yet working) prototype of attachment storage in Cassandra upon which we might one day model large object storage in the RDBMS. https://github.com/cjdelisle/xwiki-newstore/blob/master/xwiki-platform-store... Everything is handled in 1MB chunks and each chunk is flushed to the data store before the next chunk is created preventing them from building up in memory. The "version" number is used to make sure no two concurrent save operations can ever cause the attachment to end in an inconsistent state even in a non-transactional data store. Thanks, Caleb On 08/09/2012 04:43 AM, Paul Libbrecht wrote:
Le 9 août 2012 à 10:28, Caleb James DeLisle a écrit :
For loading an attachment piecewise, OutputStreams are not the only possible solution but they are (in my analysis) the best.
I absolutely agree to this.
I find the usage of byte arrays should be even deprecated (still doable while staying in 4.x?). I've been bragging about it from time to time... but could never contribute to it indeed. HttpClient does complain everytime you try to allocate entities in byte-arrays. Maybe that's too explicit but that's effective to force the developer into finding alternate solutions.
I've seen things slightly more explicit than outputstream and inputstream floating around, things related to FileUpload if I remember well. In particular, I think it is best practice to carry the length as far away into the API as possible thus allowing the best buffering and storage strategy.
Paul _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On 08/09/2012 04:28 AM, Caleb James DeLisle wrote:
My changes are in a the feature-attachment-outputstream branch https://github.com/xwiki/xwiki-platform/commits/feature-attachment-outputstr...
IMO the Hibernate blob handling is never really going to work in a cross-platform way. Even if that is not true, it absolutely won't work with Cassandra.
For loading an attachment piecewise, OutputStreams are not the only possible solution but they are (in my analysis) the best.
If we hold off on adding the API because there is no consumer in platform proper, I'll have to code around it and find another (inferior) solution and if piecewise attachment loading ever does reach platform proper, everything will be developed and debugged around inferior solution.
That's my case for it, let me know if you have any specific concerns or if your veto is on the general principle. I want to get an idea of whether this should be cleaned up or killed quickly.
Thanks,
Caleb
After discussing with Caleb on IRC [1], I've come to the conclusion that while the OutputStream method isn't mandatory, it would make things easier for the NoSQL engines which don't actually have data streaming at their core. So, working with byte array chunks is going to be a (the?) way of supporting large attachments. And gathering a sequence of BlobChunks and serializing them into an attachment works better with an OutputStream where they can be written. So, I'm re-changing my vote to +1 for the getContentOutputStream(). [1] http://dev.xwiki.org/xwiki/bin/view/IRC/xwikiArchive20120810
On 08/07/2012 11:31 PM, Caleb James DeLisle wrote:
On 08/07/2012 10:52 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:44 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:43 PM, Denis Gervalle wrote:
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream()
+1 as well.
Actually, one thing I don't like is that the API is getting even bigger, with lots of different ways of doing the same thing. Ideally, there should be only one way of providing the content, and only one way of retrieving it. And IMO working with byte streams is the right way, and I think that the current setContent(InputStream) method is the one that's best.
Returning an OutputStream where the caller can write the content is opening the door to lots of potential problems. Providing such a method becomes API, and APIs should be well designed. And an OutputStream in my head means some implicit constraints that must be well documented:
I can answer for the implementation which I wrote as a result of this discussion.
- should the stream be closed at the end?
Yes, if it is not closed then the content does not become "live".
- must the response be written before a transaction ends?
It doesn't matter when it is written since it's just dealing with FileItems.
- what if nothing is written in the stream, does that mean that the old content is preserved, or that the attachment is going to be truncated?
The attachment becomes is made empty. It is legal to have a 0 byte file.
Basically, this is a _push_ API, and I for one prefer _pull_ APIs, since the backend will get the data when it needs it, it doesn't have to document how long it's going to wait for something to be pushed.
I agree that our current way of dealing with Hibernate is not right. We should use proper blob streams for big data, like attachment content. And Hibernate's LobCreator expects a Blob object that offers access to its content via an InputStream that can be read. Again, Hibernate expects an input stream from which it will read when it needs to, it doesn't allow you to write the content into an OutputStream when you want.
There are a few issues here. When I investigated this, Hibernate's blob handling was (still is?) just a wrapper around blob management in the database which basically doesn't work. Oracle has a proprietary blob storage method which works with streams, I think Hibernate does not support this though. As I remember, mysql and postgres have blob APIs but they buffer everything in ram making it nonsensical. This was investigated to solve attachment memory consumption issues and it was concluded that it does not work, leading to FS attachment store.
A solution which will work is to piece out the attachment and store each piece separately, flushing the PersistenceManager or Session between save/load operations. they can be run in discrete transactions as long as the metadata contains a pointer to the pieces and the old pieces are not removed until after all of the new pieces are saved. On the load, you are loading a list of attachment pieces one by one and their content must be written out before it is removed from memory.
The options for making this work are:
* copy the content into a temp file then copy it again the InputStream from that file. This double buffers the content. * use PipedOutputStream to bounce it between two threads. (which I am doing now but need to debug a problem with it.) * extend XWikiAttachmentContent for this use case. * add a method to XWikiAttachmentContent which allows for writing to it using an OutputStream.
Since this pattern would be applicable to Hibernate and the technology which came from Cassandra attachments could be ported into the main trunk, I wanted to do the latter. If you are not swayed then I will return to evaluating the former methods.
Caleb
So, consider this a -1 until you can convince me that it's indeed something we want to include in our APIs.
+0 for getOutputStream() -0 for other previous proposals.
On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
getOutputStream() is not very descriptive although I suppose a good javadoc comment would alleviate the issue, I wrestled with the name myself and settled on setContent() because it overloads the existing setContent() so it should be a bit easier to remember.
If you guys like getOutputStream(), I'm happy enough with it.
Caleb
On 08/07/2012 03:24 AM, Thomas Mortagne wrote: > +1 for the idea in general but same comment than Marius > > On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea > <[email protected]> wrote: >> I understand the need and I'm +1 but I don't like the method name >> (neither setContent() nor addContent()). WDYT about getOutputStream() >> ? >> >> Thanks, >> Marius >> >> On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle >> <[email protected]> wrote: >>> Hi, >>> In the development of Cassandra attachments, I found I want to >>> load an attachment one chunk at a time and >>> write that chunk to a provided OutputStream, this is how I envision next generation Hibernate attachments working too. >>> >>> I would like to add to XWikiAttachmentContent: >>> >>> public OutputStream addContent(); >>> >>> which returns an OutputStream that allows writing the attachment content and upon close, >>> sets the attachment content as dirty and resets the size field. >>> >>> WDYT? >>> >>> Caleb
-- Sergiu Dumitriu http://purl.org/net/sergiu/
Ok, 5 +1 including myself, no 0, no -1 Merging it now, thanks for the review. Caleb On 08/09/2012 09:54 PM, Sergiu Dumitriu wrote:
On 08/09/2012 04:28 AM, Caleb James DeLisle wrote:
My changes are in a the feature-attachment-outputstream branch https://github.com/xwiki/xwiki-platform/commits/feature-attachment-outputstr...
IMO the Hibernate blob handling is never really going to work in a cross-platform way. Even if that is not true, it absolutely won't work with Cassandra.
For loading an attachment piecewise, OutputStreams are not the only possible solution but they are (in my analysis) the best.
If we hold off on adding the API because there is no consumer in platform proper, I'll have to code around it and find another (inferior) solution and if piecewise attachment loading ever does reach platform proper, everything will be developed and debugged around inferior solution.
That's my case for it, let me know if you have any specific concerns or if your veto is on the general principle. I want to get an idea of whether this should be cleaned up or killed quickly.
Thanks,
Caleb
After discussing with Caleb on IRC [1], I've come to the conclusion that while the OutputStream method isn't mandatory, it would make things easier for the NoSQL engines which don't actually have data streaming at their core. So, working with byte array chunks is going to be a (the?) way of supporting large attachments. And gathering a sequence of BlobChunks and serializing them into an attachment works better with an OutputStream where they can be written.
So, I'm re-changing my vote to +1 for the getContentOutputStream().
[1] http://dev.xwiki.org/xwiki/bin/view/IRC/xwikiArchive20120810
On 08/07/2012 11:31 PM, Caleb James DeLisle wrote:
On 08/07/2012 10:52 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:44 PM, Sergiu Dumitriu wrote:
On 08/07/2012 03:43 PM, Denis Gervalle wrote:
From the current interface, I would use getContentOutputStream(), since this would be the opposite of getContentInputStream(). This seems to me very descriptive compare to a setContent() returning an OutputStream, since a set is not supposed to return anything. I would use setContent(OutputStream) if it was your goal, but this probably not what you expect.
So, +1 for getContentOutputStream()
+1 as well.
Actually, one thing I don't like is that the API is getting even bigger, with lots of different ways of doing the same thing. Ideally, there should be only one way of providing the content, and only one way of retrieving it. And IMO working with byte streams is the right way, and I think that the current setContent(InputStream) method is the one that's best.
Returning an OutputStream where the caller can write the content is opening the door to lots of potential problems. Providing such a method becomes API, and APIs should be well designed. And an OutputStream in my head means some implicit constraints that must be well documented:
I can answer for the implementation which I wrote as a result of this discussion.
- should the stream be closed at the end?
Yes, if it is not closed then the content does not become "live".
- must the response be written before a transaction ends?
It doesn't matter when it is written since it's just dealing with FileItems.
- what if nothing is written in the stream, does that mean that the old content is preserved, or that the attachment is going to be truncated?
The attachment becomes is made empty. It is legal to have a 0 byte file.
Basically, this is a _push_ API, and I for one prefer _pull_ APIs, since the backend will get the data when it needs it, it doesn't have to document how long it's going to wait for something to be pushed.
I agree that our current way of dealing with Hibernate is not right. We should use proper blob streams for big data, like attachment content. And Hibernate's LobCreator expects a Blob object that offers access to its content via an InputStream that can be read. Again, Hibernate expects an input stream from which it will read when it needs to, it doesn't allow you to write the content into an OutputStream when you want.
There are a few issues here. When I investigated this, Hibernate's blob handling was (still is?) just a wrapper around blob management in the database which basically doesn't work. Oracle has a proprietary blob storage method which works with streams, I think Hibernate does not support this though. As I remember, mysql and postgres have blob APIs but they buffer everything in ram making it nonsensical. This was investigated to solve attachment memory consumption issues and it was concluded that it does not work, leading to FS attachment store.
A solution which will work is to piece out the attachment and store each piece separately, flushing the PersistenceManager or Session between save/load operations. they can be run in discrete transactions as long as the metadata contains a pointer to the pieces and the old pieces are not removed until after all of the new pieces are saved. On the load, you are loading a list of attachment pieces one by one and their content must be written out before it is removed from memory.
The options for making this work are:
* copy the content into a temp file then copy it again the InputStream from that file. This double buffers the content. * use PipedOutputStream to bounce it between two threads. (which I am doing now but need to debug a problem with it.) * extend XWikiAttachmentContent for this use case. * add a method to XWikiAttachmentContent which allows for writing to it using an OutputStream.
Since this pattern would be applicable to Hibernate and the technology which came from Cassandra attachments could be ported into the main trunk, I wanted to do the latter. If you are not swayed then I will return to evaluating the former methods.
Caleb
So, consider this a -1 until you can convince me that it's indeed something we want to include in our APIs.
+0 for getOutputStream() -0 for other previous proposals.
On Tue, Aug 7, 2012 at 9:01 PM, Caleb James DeLisle < [email protected]> wrote:
> getOutputStream() is not very descriptive although I suppose a good > javadoc comment would alleviate > the issue, I wrestled with the name myself and settled on setContent() > because it overloads the existing setContent() so it should be a bit > easier to remember. > > If you guys like getOutputStream(), I'm happy enough with it. > > Caleb > > > On 08/07/2012 03:24 AM, Thomas Mortagne wrote: >> +1 for the idea in general but same comment than Marius >> >> On Tue, Aug 7, 2012 at 7:35 AM, Marius Dumitru Florea >> <[email protected]> wrote: >>> I understand the need and I'm +1 but I don't like the method name >>> (neither setContent() nor addContent()). WDYT about getOutputStream() >>> ? >>> >>> Thanks, >>> Marius >>> >>> On Tue, Aug 7, 2012 at 12:06 AM, Caleb James DeLisle >>> <[email protected]> wrote: >>>> Hi, >>>> In the development of Cassandra attachments, I found I want to >>>> load an > attachment one chunk at a time and >>>> write that chunk to a provided OutputStream, this is how I envision > next generation Hibernate attachments working too. >>>> >>>> I would like to add to XWikiAttachmentContent: >>>> >>>> public OutputStream addContent(); >>>> >>>> which returns an OutputStream that allows writing the attachment > content and upon close, >>>> sets the attachment content as dirty and resets the size field. >>>> >>>> WDYT? >>>> >>>> Caleb
participants (6)
-
Caleb James DeLisle -
Denis Gervalle -
Marius Dumitru Florea -
Paul Libbrecht -
Sergiu Dumitriu -
Thomas Mortagne