Ah, I see, the problem was that you can call seek between the writeAtMost calls, which will do the wrong thing if dirty_ is not accurate.
So, CASSANDRA-7 should be marked closed now. -Jonathan On Fri, Mar 20, 2009 at 9:16 AM, Sandeep Tata <[email protected]> wrote: > It is an equivalent fix -- you can either set the dirty_ bit inside > writeAtMost after the arraycopy like I did in my patch, or set it > after calling writeAtMost here. > > I thought of doing this, but then, everytime you use writeAtMost you > have to remember to set this bit -- I think it is better to do this > inside writeAtMost. > > As far as I can see, the rest of the changes are comment deletions and > indentation changes, right? > > > On Fri, Mar 20, 2009 at 7:27 AM, Jonathan Ellis <[email protected]> wrote: >> it's actually not just indentation, it's brace placement -- from >> >> while (len > 0) >> { >> int n = this.writeAtMost(b, off, len); >> off += n; >> len -= n; >> } >> this.dirty_ = true; >> >> to >> >> while (len > 0) >> { >> int n = this.writeAtMost(b, off, len); >> off += n; >> len -= n; >> this.dirty_ = true; >> } >> >> which is more correct if an empty byte[] is passed in (it won't set dirty_). >> >> CASSANDRA-7 was not addressed that I could see. >> >> On Fri, Mar 20, 2009 at 1:11 AM, Sandeep Tata <[email protected]> wrote: >>> Folks, >>> >>> It is easier for the community to follow changes to the code if we use >>> JIRAs and understand what bugs/features each of the patches address >>> instead of simply committing changes. >>> >>> Does this patch address any issues other than CASSANDRA-7 >>> (https://issues.apache.org/jira/browse/CASSANDRA-7) and the patch >>> there? I only see formatting changes otherwise... >>> >>> In general, it is not good practice to mix formatting changes >>> (indentation) along with critical code changes. Keeping them separate >>> makes it easier for the community to understand the effect of each >>> patch. I understand this is a young open source project, and we're all >>> learning -- but simple things like this can really help other >>> contributors keep track of the code as it evolves. >>> >>> Sandeep >>> >>> >>> On Thu, Mar 19, 2009 at 12:48 PM, <[email protected]> wrote: >>>> Author: alakshman >>>> Date: Thu Mar 19 19:48:21 2009 >>>> New Revision: 756155 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=756155&view=rev >>>> Log: >>>> Fixed some bugs that resulted from moving sources over. >>>> >>>> Modified: >>>> >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/AIORandomAccessFile.java >>>> >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/BufferedRandomAccessFile.java >>>> >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/ChecksumRandomAccessFile.java >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/SSTable.java >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/SequenceFile.java >>>> >>>> Modified: >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/AIORandomAccessFile.java >>>> URL: >>>> http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/org/apache/cassandra/io/AIORandomAccessFile.java?rev=756155&r1=756154&r2=756155&view=diff >>>> ============================================================================== >>>> --- >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/AIORandomAccessFile.java >>>> (original) >>>> +++ >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/AIORandomAccessFile.java >>>> Thu Mar 19 19:48:21 2009 >>>> @@ -656,50 +656,6 @@ >>>> this.curr_ += len; >>>> return len; >>>> } >>>> - >>>> - public static void main(String[] args) throws Throwable >>>> - { >>>> - /* >>>> - int i = 0; >>>> - try >>>> - { >>>> - RandomAccessFile aRaf2 = new AIORandomAccessFile( new >>>> File("/var/cassandra/test.dat"), 64*1024); >>>> - aRaf2.seek(0L); >>>> - while ( i < 10000 ) >>>> - { >>>> - aRaf2.writeInt(32); >>>> - aRaf2.writeUTF("Avinash Lakshman"); >>>> - ++i; >>>> - } >>>> - aRaf2.close(); >>>> - } >>>> - catch( IOException ex ) >>>> - { >>>> - ex.printStackTrace(); >>>> - } >>>> - */ >>>> - /* >>>> - int j = 0; >>>> - try >>>> - { >>>> - RandomAccessFile aRaf2 = new AIORandomAccessFile( new >>>> File("/var/cassandra/test.dat") ); >>>> - while ( j < 10 ) >>>> - { >>>> - System.out.println( aRaf2.readInt() ); >>>> - System.out.println( aRaf2.readUTF() ); >>>> - ++j; >>>> - } >>>> - aRaf2.close(); >>>> - } >>>> - catch( IOException ex ) >>>> - { >>>> - ex.printStackTrace(); >>>> - } >>>> - */ >>>> - >>>> - ExecutorService es = new ContinuationsExecutor(1, 1, >>>> Integer.MAX_VALUE, TimeUnit.SECONDS, new LinkedBlockingQueue<Runnable>() ); >>>> - es.execute(new ReadImpl()); >>>> - } >>>> } >>>> >>>> class ReadImpl implements Runnable >>>> >>>> Modified: >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/BufferedRandomAccessFile.java >>>> URL: >>>> http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/org/apache/cassandra/io/BufferedRandomAccessFile.java?rev=756155&r1=756154&r2=756155&view=diff >>>> ============================================================================== >>>> --- >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/BufferedRandomAccessFile.java >>>> (original) >>>> +++ >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/BufferedRandomAccessFile.java >>>> Thu Mar 19 19:48:21 2009 >>>> @@ -165,20 +165,20 @@ >>>> * disk. If the file was created read-only, this method is a no-op. >>>> */ >>>> public void flush() throws IOException >>>> - { >>>> + { >>>> this.flushBuffer(); >>>> } >>>> >>>> /* Flush any dirty bytes in the buffer to disk. */ >>>> private void flushBuffer() throws IOException >>>> - { >>>> + { >>>> if (this.dirty_) >>>> { >>>> if (this.diskPos_ != this.lo_) >>>> super.seek(this.lo_); >>>> int len = (int) (this.curr_ - this.lo_); >>>> super.write(this.buff_, 0, len); >>>> - this.diskPos_ = this.curr_; >>>> + this.diskPos_ = this.curr_; >>>> this.dirty_ = false; >>>> } >>>> } >>>> @@ -222,7 +222,7 @@ >>>> { >>>> if (pos >= this.hi_ || pos < this.lo_) >>>> { >>>> - // seeking outside of current buffer -- flush and read >>>> + // seeking outside of current buffer -- flush and read >>>> this.flushBuffer(); >>>> this.lo_ = pos & BuffMask_; // start at BuffSz boundary >>>> this.maxHi_ = this.lo_ + (long) this.buff_.length; >>>> @@ -332,14 +332,14 @@ >>>> } >>>> >>>> public void write(byte[] b, int off, int len) throws IOException >>>> - { >>>> + { >>>> while (len > 0) >>>> - { >>>> + { >>>> int n = this.writeAtMost(b, off, len); >>>> off += n; >>>> len -= n; >>>> - } >>>> - this.dirty_ = true; >>>> + this.dirty_ = true; >>>> + } >>>> } >>>> >>>> /* >>>> @@ -347,7 +347,7 @@ >>>> * the number of bytes written. >>>> */ >>>> private int writeAtMost(byte[] b, int off, int len) throws IOException >>>> - { >>>> + { >>>> if (this.curr_ >= this.hi_) >>>> { >>>> if (this.hitEOF_ && this.hi_ < this.maxHi_) >>>> @@ -356,8 +356,8 @@ >>>> this.hi_ = this.maxHi_; >>>> } >>>> else >>>> - { >>>> - // slow path -- write current buffer; read next one >>>> + { >>>> + // slow path -- write current buffer; read next one >>>> this.seek(this.curr_); >>>> if (this.curr_ == this.hi_) >>>> { >>>> >>>> Modified: >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/ChecksumRandomAccessFile.java >>>> URL: >>>> http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/org/apache/cassandra/io/ChecksumRandomAccessFile.java?rev=756155&r1=756154&r2=756155&view=diff >>>> ============================================================================== >>>> --- >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/ChecksumRandomAccessFile.java >>>> (original) >>>> +++ >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/ChecksumRandomAccessFile.java >>>> Thu Mar 19 19:48:21 2009 >>>> @@ -391,8 +391,8 @@ >>>> int n = this.writeAtMost(b, off, len); >>>> off += n; >>>> len -= n; >>>> + this.dirty_ = true; >>>> } >>>> - this.dirty_ = true; >>>> } >>>> >>>> /* >>>> >>>> Modified: >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/SSTable.java >>>> URL: >>>> http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/org/apache/cassandra/io/SSTable.java?rev=756155&r1=756154&r2=756155&view=diff >>>> ============================================================================== >>>> --- incubator/cassandra/trunk/src/org/apache/cassandra/io/SSTable.java >>>> (original) >>>> +++ incubator/cassandra/trunk/src/org/apache/cassandra/io/SSTable.java Thu >>>> Mar 19 19:48:21 2009 >>>> @@ -432,7 +432,8 @@ >>>> */ >>>> public SSTable(String directory, String filename, PartitionerType >>>> pType) throws IOException >>>> { >>>> - dataFile_ = directory + System.getProperty("file.separator") + >>>> filename + "-Data.db"; >>>> + dataFile_ = directory + System.getProperty("file.separator") + >>>> filename + "-Data.db"; >>>> + // dataWriter_ = SequenceFile.writer(dataFile_); >>>> dataWriter_ = SequenceFile.bufferedWriter(dataFile_, 4*1024*1024); >>>> // dataWriter_ = SequenceFile.chksumWriter(dataFile_, 4*1024*1024); >>>> SSTable.positionAfterFirstBlockIndex_ = >>>> dataWriter_.getCurrentPosition(); >>>> @@ -747,22 +748,7 @@ >>>> SSTable.indexMetadataMap_.put(dataFile_, keyPositionInfos); >>>> } >>>> >>>> - keyPositionInfos.add(new KeyPositionInfo(blockIndex.firstKey(), >>>> position)); >>>> - /* >>>> - try >>>> - { >>>> - keyPositionInfos.add(new >>>> KeyPositionInfo(blockIndex.firstKey(), position)); >>>> - } >>>> - catch(Exception ex) >>>> - { >>>> - Set<String> keysInBlock = blockIndex.keySet(); >>>> - for( String keyInBlock : keysInBlock ) >>>> - { >>>> - logger_.warn("BLOCK KEY: " + keyInBlock); >>>> - } >>>> - logger_.warn(LogUtil.throwableToString(ex)); >>>> - } >>>> - */ >>>> + keyPositionInfos.add(new KeyPositionInfo(blockIndex.firstKey(), >>>> position)); >>>> blockIndex.clear(); >>>> } >>>> >>>> @@ -1088,9 +1074,8 @@ >>>> /* reset the buffer and serialize the Bloom Filter. */ >>>> DataOutputBuffer bufOut = new DataOutputBuffer(); >>>> BloomFilter.serializer().serialize(bf, bufOut); >>>> - bufOut.close(); >>>> - >>>> close(bufOut.getData(), bufOut.getLength()); >>>> + bufOut.close(); >>>> // byte[] bytes = new byte[bufOut.getLength()]; >>>> // System.arraycopy(bufOut.getData(), 0, bytes, 0, >>>> bufOut.getLength()); >>>> // close(bytes, bytes.length); >>>> >>>> Modified: >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/SequenceFile.java >>>> URL: >>>> http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/org/apache/cassandra/io/SequenceFile.java?rev=756155&r1=756154&r2=756155&view=diff >>>> ============================================================================== >>>> --- >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/SequenceFile.java >>>> (original) >>>> +++ >>>> incubator/cassandra/trunk/src/org/apache/cassandra/io/SequenceFile.java >>>> Thu Mar 19 19:48:21 2009 >>>> @@ -974,7 +974,7 @@ >>>> >>>> Coordinate coordinate = columnRange.coordinate(); >>>> /* seek to the correct offset to the data, >>>> and calculate the data size */ >>>> - file_.skipBytes((int)coordinate.start_); >>>> + file_.skipBytes((int)coordinate.start_); >>>> dataSize = (int)(coordinate.end_ - >>>> coordinate.start_); >>>> >>>> /* >>>> >>>> >>>> >>> >> >
