Skip to content
This repository was archived by the owner on Apr 24, 2026. It is now read-only.

Set the default buffer size (if not specified) to 128k#66

Open
childsb wants to merge 6 commits into
gluster:masterfrom
childsb:default_buffer
Open

Set the default buffer size (if not specified) to 128k#66
childsb wants to merge 6 commits into
gluster:masterfrom
childsb:default_buffer

Conversation

@childsb

@childsb childsb commented Nov 18, 2013

Copy link
Copy Markdown
Contributor

If no configuration value is specified for io.file.buffer.size, set it to 128k.

@jayunit100

Copy link
Copy Markdown
Contributor

preapproved +1 , pending addition of a Log.info("Setting buffer size to ...........") statement. otherwise this is really hard to debug at scale in case any issues.

@wattsteve

Copy link
Copy Markdown

I'd like to see unit tests attached to each pull request. Perhaps, take Jay's pull 55 and merge it with this?

@jayunit100

Copy link
Copy Markdown
Contributor

good point steve :) Retracting my +1 until you merge my unit test :) :) #65 (or just copy the code - i dont care about the pull request, just copy the code into this commit )

@childsb

childsb commented Nov 18, 2013

Copy link
Copy Markdown
Contributor Author

@jayunit100 @wattsteve updated this pull with your requests.

@wattsteve

Copy link
Copy Markdown

+1

@jayunit100

Copy link
Copy Markdown
Contributor

nice one brad +1. real quick before you merge: can you confirm that the unit test passes now ? I've confirmed that it fails on the old shim which uses a 4 or 8K default.

@childsb

childsb commented Nov 18, 2013

Copy link
Copy Markdown
Contributor Author

@jayunit100 : that unit test isn't passing. can you fix + submit a change?

@jayunit100

Copy link
Copy Markdown
Contributor

hmmmmmmm okay will patch it up . you want me to commit to your childsb/branch ?

@jayunit100

Copy link
Copy Markdown
Contributor

hola. The test is failing for all the right reasons: The default buffering is still 4096 bytes. :)... You need to update your "if ..." statement to override the default of 4096, not just to override in case of null/empty. Hadoop will provide a very low 4096 byte default, and that will (Rightly) cause this test to fail.

@jayunit100

Copy link
Copy Markdown
Contributor

Well, nevermind. I guess i will patch the test so that it confirms that the io.file.buffer.size parameter is honored and let you folks work out the details of wether or not we should even ALLOW the 4K byte write buffer, or just override it entirely.

@jayunit100

Copy link
Copy Markdown
Contributor
//this test will pass, edit (bufferSize) to be larger, once we decide what gluster should allow as a minium.
@Test
public void testBufferSpill() throws Exception {
    Path out = new Path("a");
    final Integer buffersize = fs.getConf().getInt("io.file.buffer.size", -1);
    FSDataOutputStream os = fs.create(out);

    int written=0;
    /**
     * Assert that writes smaller than 10KB are NOT spilled to disk
     */
    while(written<buffersize){
        os.write("ASDF".getBytes());
        written+="ASDF".getBytes().length;
        //now, we expect
        Assert.assertTrue("asserting that file not written yet...",fs.getLength(out)==0);
    }
    os.flush();
    Assert.assertTrue("asserting that is now written... ",fs.getLength(out)>=buffersize);

    os.close();
    fs.delete(out);
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants