[logback-dev] [JIRA] Commented: (LBCORE-128) Please support implementation of binary log files in RollingFileAppender/FileAppender

[Joern Huxhorn]

On 17.02.2010, at 10:53, Maarten Bosteels wrote:

> Hi Joern,
>
> The reason for using generics in the Encoder interface is to avoid  
> having to cast.  (hmm, I guess that's always the reason to use  
> generics ?)
>
> Take for example, the ObjectEncoder: it needs an ObjectOutputStream,  
> so it wraps the given OutputStream in an ObjectOutputStream:
>
> public class ObjectEncoder implements Encoder<ObjectOutputStream> {
>
>   public void encode(ILoggingEvent event, ObjectOutputStream output)  
> throws IOException {
>     output.writeObject(event);
>   }
>
>   public ObjectOutputStream decorate(OutputStream os) throws  
> IOException {
>     return new ObjectOutputStream(os);
>   }
> }
>
>
> On the other hand, the ProtobufEncoder can use a plain OutputStream,  
> so its decorate impl is simply a no-op:
>
> public class ProtobufEncoder implements Encoder<OutputStream>{
>
>   private ProtoBufConvertor convertor = new ProtoBufConvertor();
>
>   public void encode(ILoggingEvent event, OutputStream output)  
> throws IOException {
>     LoggingProtos.LoggingEvent ev = convertor.convert(event);
>     ev.writeTo(output);
>   }
>
>   public OutputStream decorate(OutputStream os) throws IOException {
>     return os;
>   }
> }
>
> Clear now ?

Yes, understood.

But if I take a look at
http://code.google.com/p/firewood/source/browse/trunk/compare-formats/src/test/java/com/googlecode/firewood/encoder/SocketAppender.java

...I'm not sure why SocketAppender should be concerned about the  
implementation of the encoder, or, more precisely, about the stream  
type that is required by the encoder implementation.

I also don't know if this would be of any help during appender  
initialization via Joran since the generic type will be erased.

If I understand everything correctly then decorate is called once  
during startup and the decorated stream is reused.
This could become problematic since not every encoder can be  
implemented like this.

If Java serialization is used and events should be randomly accessible  
without reading all previous events first, then a new  
ObjectOutputStream must be created for every event that is written.  
Otherwise they depend on previously written events.
The same is the case for GZIPInputStream/OutputStream.

Cheers,
Jörn.


>
> Maarten
>
>
> On Tue, Feb 16, 2010 at 11:18 PM, Joern Huxhorn (JIRA) <noreply-jira at qos.ch 
> > wrote:
>
>    [ http://jira.qos.ch/browse/LBCORE-128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11533#action_11533 
>  ]
>
> Joern Huxhorn commented on LBCORE-128:
> --------------------------------------
>
> My branch was meant to implement what I need in a manner of least  
> possible impact.
> I detached the implementation of FileAppender from WriterAppender,  
> extending UnsynchronizedAppenderBase instead.
>
> writerWrite(String s, boolean flush) (which I didn't rename even  
> though there's no writer anymore but a stream instead) is now using  
> convertToBytes(String s) and writeBytes(byte[] bytes, boolean  
> flush), i.e. the appender writes bytes into a stream instead of  
> writing a String into a Writer. Encoding of the String isn't handled  
> by the Writer anymore. Instead, String.getBytes(charset) is used to  
> transform into bytes. The charset is initialized once during startup.
>
> I've also taken a look at Maartens Encoder but I fear I don't really  
> get the reason for either the decorate method or the use of generics  
> in that case.
>
> In the meantime, I've also implemented streaming encoder/decoder  
> interfaces for sulky/Lilith.
> http://github.com/huxi/sulky/tree/master/sulky-codec/src/main/java/de/huxhorn/sulky/codec/streaming/
> Those are a supplement of my previous byte[]-based encoders/decoders  
> and allow chaining. I use generics to define the type that is  
> encoded/decoded. The decoration of the stream is meant to be  
> encapsulated in the actual implementation.
>
> I realize that this does not allow reusing of the same encapsulated  
> stream, though. This means that encoding using Java serialization is  
> less efficient since duplicate strings, for example, are serialized  
> over and over instead of only once, referencing the already  
> serialized string.
> It has the advantage that written byte[] are not dependent on  
> previously written ones, on the other hand. This is crucial for  
> random event access, in my case.
>
> I don't say Maartens design is bad and mine is better. Neither you  
> nor (especially!) Maarten should get this impression.
>
> If you are actually aiming for a bigger change than my small patch  
> then it might be a good idea to keep the layout concept, even for  
> binary files, and add the ability to use binary layouts that would  
> also contain the encoder for the event.
> I've enabled binary headers and footers by declaring writeHeader() &  
> writeFooter() protected instead of private in FileAppender - so I  
> could (re)implement them as needed in my extending class.
>
> If Layout would/could handle binary data for header, footer and  
> actual events instead, extending RollingFileAppender wouldn't be  
> necessary anymore. One could simply declare a LilithLayout (with  
> parameters?) instead.
> This would be a much cleaner design but I feared that such a  
> fundamental change wouldn't have any chance of being accepted for  
> inclusion.
>
> Thanks for taking a look at this issue. I really appreciate it!
>
> > Please support implementation of binary log files in  
> RollingFileAppender/FileAppender
> >  
> -------------------------------------------------------------------------------------
> >
> >                 Key: LBCORE-128
> >                 URL: http://jira.qos.ch/browse/LBCORE-128
> >             Project: logback-core
> >          Issue Type: Improvement
> >          Components: Appender
> >    Affects Versions: 0.9.17
> >            Reporter: Joern Huxhorn
> >            Assignee: Ceki Gulcu
> >
> > This was discussed briefly at http://marc.info/?l=logback-dev&m=124905434331308&w=2 
>  and I forgot to file a ticket about this.
> > Currently, RandomFileAppender => FileAppender => WriterAppender is  
> using the following method in WriterAppender to actually write the  
> data:
> > protected void writerWrite(String s, boolean flush) throws  
> IOException
> > Please add an additional method like
> > protected void writerWrite(byte[] bytes, boolean flush) throws  
> IOException
> > to write to the underlying stream directly.
> > writerWrite(String, boolean) could call that method after  
> performing the transformation internally, making this change  
> transparent for the rest of the implementation.
> > Using a binary format for logfiles could have tremendous  
> performance impact as can be seen here: http://sourceforge.net/apps/trac/lilith/wiki/SerializationPerformance
>
> --
> This message is automatically generated by JIRA.
> -
> If you think it was sent incorrectly contact one of the  
> administrators: http://jira.qos.ch/secure/Administrators.jspa
> -
> For more information on JIRA, see: http://www.atlassian.com/software/jira
>
>
> _______________________________________________
> logback-dev mailing list
> logback-dev at qos.ch
> http://qos.ch/mailman/listinfo/logback-dev
>
> _______________________________________________
> logback-dev mailing list
> logback-dev at qos.ch
> http://qos.ch/mailman/listinfo/logback-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://qos.ch/pipermail/logback-dev/attachments/20100217/f708159f/attachment.html>