velox icon indicating copy to clipboard operation
velox copied to clipboard

Published 20 hours ago • verified publisher icon facebookincubator

Improve the FlushPolicyFactory to make the client can specify flush policy

Open JkSelf opened this issue 11 months ago • 10 comments

As the discussions in https://github.com/facebookincubator/velox/issues/8684, Spark and Presto have different behaviors for FlushPolicy. In order to allow different clients to configure their own FlushPolicy, we have added a flushPolicy parameter in HiveInsertTableHandle. This allows different clients to set their own flushPolicy when creating a HiveInsertTableHandle.

JkSelf avatar Feb 27 '24 05:02 JkSelf

Deploy Preview for meta-velox canceled.

Name Link
Latest commit 7b48146c5f302cccf667e81980088f7dd588a1e4
Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66d1730f80487f000878e44a

netlify[bot] avatar Feb 27 '24 05:02 netlify[bot]

@majetideepak @mbasmanova Can you help to review? Thanks.

JkSelf avatar Feb 28 '24 01:02 JkSelf

@majetideepak Thanks for your review. I have resolved all your comments. Can you help to review again? Thanks.

JkSelf avatar Feb 28 '24 08:02 JkSelf

@majetideepak @jinchengchenghh Sorry for the delay response. Resolved all your comments. Can you help to review again? Thanks for your help.

JkSelf avatar Mar 12 '24 10:03 JkSelf

@majetideepak I apologize for the delayed response. I have resolved all your comments. Can you help to review again? Thanks.

JkSelf avatar Apr 18 '24 10:04 JkSelf

@majetideepak Can you help to review again? Thanks.

JkSelf avatar Apr 23 '24 08:04 JkSelf

@JkSelf This fell through. I will review this tomorrow.

majetideepak avatar Apr 29 '24 20:04 majetideepak

@majetideepak I apologize for the late response as I was on vacation recently. I have fixed the last two comments you mentioned. Can you help to review again?

JkSelf avatar May 09 '24 07:05 JkSelf

@xiaoxmeng can you take another look at this change? Thanks!

majetideepak avatar May 09 '24 15:05 majetideepak

@HuamengJiang Can you help to review? Thanks.

JkSelf avatar May 13 '24 01:05 JkSelf

@mbasmanova Can you help to review this PR? Thanks.

JkSelf avatar May 15 '24 01:05 JkSelf

@xiaoxmeng can you help to take a look? The PR can add the rowgroup config for parquet writter. Otherwise each file has single rowgroup.

FelixYBW avatar May 15 '24 06:05 FelixYBW

@mbasmanova The failed CI seems not related with this PR. Can you help to review again? Thanks.

JkSelf avatar May 27 '24 10:05 JkSelf

The failed CI seems not related with this PR.

@JkSelf What are the failures? Would you create GitHub issues for these?

mbasmanova avatar May 28 '24 13:05 mbasmanova

@mbasmanova I will make another pass.

majetideepak avatar May 30 '24 16:05 majetideepak

@majetideepak Resolved all your comments. Can you help to review again? Thanks.

JkSelf avatar Jun 05 '24 03:06 JkSelf

@majetideepak @xiaoxmeng @HuamengJiang Can you help to review this PR again? Thanks for your help.

JkSelf avatar Jul 09 '24 01:07 JkSelf

@mbasmanova @majetideepak @xiaoxmeng @HuamengJiang @rui-mo Resolved all your comments. Can you help to review again? Thanks.

JkSelf avatar Aug 07 '24 08:08 JkSelf

@rui-mo Thanks for your review. I have resolved all your comments. Can you help to review again? Thanks.

JkSelf avatar Aug 12 '24 06:08 JkSelf

@xiaoxmeng I have updated this PR based on your suggestions. Can you help to review again? Thanks.

JkSelf avatar Aug 13 '24 06:08 JkSelf

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Aug 28 '24 03:08 facebook-github-bot

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Aug 30 '24 06:08 facebook-github-bot

@xiaoxmeng merged this pull request in facebookincubator/velox@7af17de56f88c208f35309fe13626005e34cbf81.

facebook-github-bot avatar Aug 30 '24 19:08 facebook-github-bot

Conbench analyzed the 1 benchmark run on commit 7af17de5.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

conbench-facebook[bot] avatar Aug 30 '24 19:08 conbench-facebook[bot]