This looks good to me. I made a comment regarding the default value for this.

Ok, I updated the property name to be more clear and set the min to be
DEFAULT_BLOCK_SIZE / 10, which should allow for a lot of writers, but still
maintain a reasonable (though not most efficient) block size.

-Dan

On Wed, Feb 4, 2015 at 3:36 PM, Julien Le Dem [email protected]
wrote:

This looks good to me. I made a comment regarding the default value for
this.

—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-parquet-mr/pull/115#issuecomment-72964158
.

This looks good to me.

I'm wondering if later we should add a minimum in terms of column chunk size. I know we can't guarantee it, but if we can take the number of columns into account we would have a more accurate minimum.

For example, if you have 10 columns and 128 MB, you get on average ~12MB column chunks. But if you have 100 columns, then you get on average 1.2MB column chunks. That's a big difference, so if we can make this check schema-dependent, it might be worth doing.

That's an excellent point, but would make it a lot harder to estimate the
number of writers (and apply it globally).

When you get into this situation, you've already started to degrade your
performance. This is just a minor speed bump to keep someone from doing
something catastrophically bad.

I do like the idea for making column chunk min size an alternate setting
for better tuning where you need it, but we should probably make that a
separate JIRA.

On Thu, Feb 5, 2015 at 9:09 AM, Ryan Blue [email protected] wrote:

This looks good to me.

I'm wondering if later we should add a minimum in terms of column chunk
size. I know we can't guarantee it, but if we can take the number of
columns into account we would have a more accurate minimum.

For example, if you have 10 columns and 128 MB, you get on average ~12MB
column chunks. But if you have 100 columns, then you get on average 1.2MB
column chunks. That's a big difference, so if we can make this check
schema-dependent, it might be worth doing.

—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-parquet-mr/pull/115#issuecomment-73085019
.

This looks good to me

OK, I'll create a JIRA for capture Ryan's enhancement.

Throw me a plus and I'll give merging a shot.

On Thu, Feb 5, 2015 at 11:10 AM, Julien Le Dem [email protected]
wrote:

This looks good to me

—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-parquet-mr/pull/115#issuecomment-73106680
.

I don't think you would need to change how the number of writers is affected. Just track the schema with the most columns and use that count to estimate the smallest chunk size. I agree this can be a separate JIRA issue.

You're right that when you hit this, you're already in a bad state. That's why I think it is important to produce a prominent warning and tell everyone to change their writer architecture instead of allowing this to dial down row group size.

Ryan, I agree fully that using the column approach would be more accurate (and not really harder to implement), but I'm having a harder time coming up with a sane default value. Do we have any idea what a good minimum is for a column chunk. Also, this would effectively be an average across the table. You could still have tiny column chunks if your column data is significantly skewed.

I'll take a quick look at switching the implementation.

@rdblue is this more inline with what you were thinking. I just used the page size for the default (i.e. average chunk size should never be smaller than a single page).

That might be too low, but possibly a good place to start.

@danielcweeks, page size is a great idea. That's already configurable and it makes no sense to have a column chunk less than a page.

+1

Thanks @rdblue! I like this a lot better.