You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/09/18 00:14:50 UTC
[GitHub] [pinot] 61yao opened a new pull request, #9422: [multistage] [enhancement] Split transferable block when the size is too large
61yao opened a new pull request, #9422:
URL: https://github.com/apache/pinot/pull/9422
Split the block size for mailbox service when the block size is too large. (Set the limit to 4M now)
Currently, this only supports row type DataBlock.
When the row size is greater max block size, send each row as a block. (we probably need to fix this when this actually happens)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9422:
URL: https://github.com/apache/pinot/pull/9422#discussion_r982727538
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxSendOperator.java:
##########
@@ -55,6 +54,10 @@
public class MailboxSendOperator extends BaseOperator<TransferableBlock> {
private static final Logger LOGGER = LoggerFactory.getLogger(MailboxSendOperator.class);
private static final String EXPLAIN_NAME = "MAILBOX_SEND";
+ // TODO: Set this value via command line flag or query option.
+ // Max block size in bytes to send content over mail box service.
+ // Set to 4M for now.
+ private static int _maxBlockSize = 4 * 1024 * 1024;
Review Comment:
this might be a dynamic computed value depending on the mailbox sender method, e.g. if using HASH_DIST and assume evenly distributed, then it should be `single server mailbox block size * # of servers`
but we can simply add a TODO and fix later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9422:
URL: https://github.com/apache/pinot/pull/9422#discussion_r981451980
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxSendOperator.java:
##########
@@ -192,14 +199,17 @@ private static List<BaseDataBlock> constructPartitionedDataBlock(BaseDataBlock d
}
}
- private void sendDataTableBlock(ServerInstance serverInstance, BaseDataBlock dataBlock)
+ private void sendDataTableBlock(ServerInstance serverInstance, TransferableBlock block)
throws IOException {
+ List<TransferableBlock> chunks = TransferableBlockUtils.splitBlock(block, _maxBlockSize);
String mailboxId = toMailboxId(serverInstance);
- SendingMailbox<Mailbox.MailboxContent> sendingMailbox = _mailboxService.getSendingMailbox(mailboxId);
- Mailbox.MailboxContent mailboxContent = toMailboxContent(mailboxId, dataBlock);
- sendingMailbox.send(mailboxContent);
- if (mailboxContent.getMetadataMap().containsKey(ChannelUtils.MAILBOX_METADATA_END_OF_STREAM_KEY)) {
- sendingMailbox.complete();
+ for (TransferableBlock chunk : chunks) {
+ SendingMailbox<Mailbox.MailboxContent> sendingMailbox = _mailboxService.getSendingMailbox(mailboxId);
+ Mailbox.MailboxContent mailboxContent = toMailboxContent(mailboxId, chunk.getDataBlock());
+ sendingMailbox.send(mailboxContent);
+ if (mailboxContent.getMetadataMap().containsKey(ChannelUtils.MAILBOX_METADATA_END_OF_STREAM_KEY)) {
+ sendingMailbox.complete();
+ }
Review Comment:
this should be done outside of the loop --> maybe we can just check against transferrable block contains a metadata block
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlock.java:
##########
@@ -156,7 +157,7 @@ public boolean isErrorBlock() {
public byte[] toBytes()
throws IOException {
- return _dataBlock.toBytes();
+ return getDataBlock().toBytes();
}
Review Comment:
Add a new API call and rename the current one:
```
public byte[] getDataBlock() {
// ...
}
public List<byte[]> getDataBlockTrunk() {
// ...
}
```
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlockUtils.java:
##########
@@ -43,4 +45,45 @@ public static TransferableBlock getErrorTransferableBlock(Map<Integer, String> e
public static boolean isEndOfStream(TransferableBlock transferableBlock) {
return transferableBlock.isEndOfStreamBlock();
}
+
+ /**
+ * Split a block into multiple block so that each block size is within maxBlockSize.
+ * Currently, we only support split for row type dataBlock.
+ * When row size is greater than maxBlockSize, we pack each row as a separate block.
+ */
+ public static List<TransferableBlock> splitBlock(TransferableBlock block, int maxBlockSize) {
Review Comment:
dont make this public and encapsulate this inside transferable block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao closed pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
61yao closed pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
URL: https://github.com/apache/pinot/pull/9422
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9422:
URL: https://github.com/apache/pinot/pull/9422#discussion_r981463125
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxSendOperator.java:
##########
@@ -192,14 +199,17 @@ private static List<BaseDataBlock> constructPartitionedDataBlock(BaseDataBlock d
}
}
- private void sendDataTableBlock(ServerInstance serverInstance, BaseDataBlock dataBlock)
+ private void sendDataTableBlock(ServerInstance serverInstance, TransferableBlock block)
throws IOException {
+ List<TransferableBlock> chunks = TransferableBlockUtils.splitBlock(block, _maxBlockSize);
String mailboxId = toMailboxId(serverInstance);
- SendingMailbox<Mailbox.MailboxContent> sendingMailbox = _mailboxService.getSendingMailbox(mailboxId);
- Mailbox.MailboxContent mailboxContent = toMailboxContent(mailboxId, dataBlock);
- sendingMailbox.send(mailboxContent);
- if (mailboxContent.getMetadataMap().containsKey(ChannelUtils.MAILBOX_METADATA_END_OF_STREAM_KEY)) {
- sendingMailbox.complete();
+ for (TransferableBlock chunk : chunks) {
+ SendingMailbox<Mailbox.MailboxContent> sendingMailbox = _mailboxService.getSendingMailbox(mailboxId);
+ Mailbox.MailboxContent mailboxContent = toMailboxContent(mailboxId, chunk.getDataBlock());
+ sendingMailbox.send(mailboxContent);
+ if (mailboxContent.getMetadataMap().containsKey(ChannelUtils.MAILBOX_METADATA_END_OF_STREAM_KEY)) {
+ sendingMailbox.complete();
+ }
Review Comment:
actually let's dont use this. just use `TransferableBlockUtils.isEndOfStream()` to check this.
the ChannelUtils. configuration is used by mailbox to check if a stream has ended (since it doesn't know how to parse the MailboxContent).
in operator it does know the mailbox content contains a transferable block so we should check with that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9422:
URL: https://github.com/apache/pinot/pull/9422#issuecomment-1255592726
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9422?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#9422](https://codecov.io/gh/apache/pinot/pull/9422?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (313062b) into [master](https://codecov.io/gh/apache/pinot/commit/3633495f5d09de36a2a3155e0e9268ec48ad05f7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3633495) will **decrease** coverage by `43.66%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #9422 +/- ##
=============================================
- Coverage 69.73% 26.06% -43.67%
+ Complexity 5087 44 -5043
=============================================
Files 1890 1890
Lines 100743 101250 +507
Branches 15349 15389 +40
=============================================
- Hits 70252 26392 -43860
- Misses 25506 72202 +46696
+ Partials 4985 2656 -2329
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `26.06% <0.00%> (+0.08%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/9422?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/query/runtime/blocks/TransferableBlock.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9ibG9ja3MvVHJhbnNmZXJhYmxlQmxvY2suamF2YQ==) | `0.00% <0.00%> (-61.91%)` | :arrow_down: |
| [...t/query/runtime/blocks/TransferableBlockUtils.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9ibG9ja3MvVHJhbnNmZXJhYmxlQmxvY2tVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ot/query/runtime/operator/MailboxSendOperator.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9NYWlsYm94U2VuZE9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [...in/java/org/apache/pinot/spi/utils/BytesUtils.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQnl0ZXNVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/user/RoleType.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3VzZXIvUm9sZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1395 more](https://codecov.io/gh/apache/pinot/pull/9422/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9422:
URL: https://github.com/apache/pinot/pull/9422#issuecomment-1258810839
> @walterddr friendly ping for review
friendly ping again :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on a diff in pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
61yao commented on code in PR #9422:
URL: https://github.com/apache/pinot/pull/9422#discussion_r981641204
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlock.java:
##########
@@ -156,7 +157,7 @@ public boolean isErrorBlock() {
public byte[] toBytes()
throws IOException {
- return _dataBlock.toBytes();
+ return getDataBlock().toBytes();
}
Review Comment:
Done
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlockUtils.java:
##########
@@ -43,4 +45,45 @@ public static TransferableBlock getErrorTransferableBlock(Map<Integer, String> e
public static boolean isEndOfStream(TransferableBlock transferableBlock) {
return transferableBlock.isEndOfStreamBlock();
}
+
+ /**
+ * Split a block into multiple block so that each block size is within maxBlockSize.
+ * Currently, we only support split for row type dataBlock.
+ * When row size is greater than maxBlockSize, we pack each row as a separate block.
+ */
+ public static List<TransferableBlock> splitBlock(TransferableBlock block, int maxBlockSize) {
Review Comment:
Done
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MailboxSendOperator.java:
##########
@@ -192,14 +199,17 @@ private static List<BaseDataBlock> constructPartitionedDataBlock(BaseDataBlock d
}
}
- private void sendDataTableBlock(ServerInstance serverInstance, BaseDataBlock dataBlock)
+ private void sendDataTableBlock(ServerInstance serverInstance, TransferableBlock block)
throws IOException {
+ List<TransferableBlock> chunks = TransferableBlockUtils.splitBlock(block, _maxBlockSize);
String mailboxId = toMailboxId(serverInstance);
- SendingMailbox<Mailbox.MailboxContent> sendingMailbox = _mailboxService.getSendingMailbox(mailboxId);
- Mailbox.MailboxContent mailboxContent = toMailboxContent(mailboxId, dataBlock);
- sendingMailbox.send(mailboxContent);
- if (mailboxContent.getMetadataMap().containsKey(ChannelUtils.MAILBOX_METADATA_END_OF_STREAM_KEY)) {
- sendingMailbox.complete();
+ for (TransferableBlock chunk : chunks) {
+ SendingMailbox<Mailbox.MailboxContent> sendingMailbox = _mailboxService.getSendingMailbox(mailboxId);
+ Mailbox.MailboxContent mailboxContent = toMailboxContent(mailboxId, chunk.getDataBlock());
+ sendingMailbox.send(mailboxContent);
+ if (mailboxContent.getMetadataMap().containsKey(ChannelUtils.MAILBOX_METADATA_END_OF_STREAM_KEY)) {
+ sendingMailbox.complete();
+ }
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr commented on a diff in pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9422:
URL: https://github.com/apache/pinot/pull/9422#discussion_r981874998
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlock.java:
##########
@@ -47,6 +48,49 @@ public class TransferableBlock implements Block {
private BaseDataBlock _dataBlock;
private List<Object[]> _container;
+ /**
+ * Split a block into multiple block so that each block size is within maxBlockSize.
+ * Currently, we only support split for row type dataBlock.
+ * For columnar and metadata data block, we return the original data block.
+ * For metadata data block, it has to be initialized through bytes instead of data block.
+ *
+ * When row size is greater than maxBlockSize, we pack each row as a separate block.
+ */
+ private List<TransferableBlock> splitBlock(int maxBlockSize) {
Review Comment:
Let's return `List<BaseDataBlock>` here
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlock.java:
##########
@@ -47,6 +48,49 @@ public class TransferableBlock implements Block {
private BaseDataBlock _dataBlock;
private List<Object[]> _container;
+ /**
+ * Split a block into multiple block so that each block size is within maxBlockSize.
+ * Currently, we only support split for row type dataBlock.
+ * For columnar and metadata data block, we return the original data block.
+ * For metadata data block, it has to be initialized through bytes instead of data block.
+ *
+ * When row size is greater than maxBlockSize, we pack each row as a separate block.
+ */
+ private List<TransferableBlock> splitBlock(int maxBlockSize) {
+ List<TransferableBlock> blockChunks = new ArrayList<>();
+ // TODO: Add support for all data block type.
+ if (getType() != BaseDataBlock.Type.ROW) {
+ blockChunks.add(this);
+ return blockChunks;
+ }
+ // TODO: Store row size in bytes inside data block.
+ DataSchema dataSchema = getDataBlock().getDataSchema();
+ int[] columnOffsets = new int[dataSchema.size()];
+ int rowSizeInBytes = DataBlockUtils.computeColumnOffsets(dataSchema, columnOffsets);
+ List<Object[]> chunk = new ArrayList<>();
+ long chunkSize = 0;
+ for (Object[] unit : getContainer()) {
+ if (rowSizeInBytes >= maxBlockSize) {
Review Comment:
if you use `rowSizeInBytes` to split chunks, you know in advance how many rows to write b/c it is fixed. no need to check every row (unit)
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlockUtils.java:
##########
@@ -27,7 +27,6 @@ public final class TransferableBlockUtils {
private TransferableBlockUtils() {
// do not instantiate.
}
-
Review Comment:
(nit) revert
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/blocks/TransferableBlock.java:
##########
@@ -154,9 +199,34 @@ public boolean isErrorBlock() {
return _isErrorBlock;
}
- public byte[] toBytes()
+ // If the data is passed in as container, only row and columnar data block type is supported.
+ public byte[] getDataBlockBytes()
throws IOException {
- return _dataBlock.toBytes();
+ return getDataBlock().toBytes();
+ }
+
+ /**
+ * Returns a list of bytes array split from data block.
+ * Each byte array size is smaller than maxBlockSize.
+ *
+ * Currently, we only support split for row type dataBlock.
+ * For columnar and metadata data block, we return the original data block.
+ * For metadata data block, it has to be initialized through bytes instead of data block.
+ *
+ * When row size is greater than maxBlockSize, we pack each row as a separate block.
+ *
+ * @param maxBlockSize
+ * @return
+ * @throws IOException
+ */
+ public List<byte[]> getChunkBytes(int maxBlockSize)
Review Comment:
```suggestion
public List<BaseDataBlock> getDataBlockChunks(int maxBlockSize)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] 61yao commented on pull request #9422: [multistage] [enhancement] Split transferable block when the size is too large
Posted by GitBox <gi...@apache.org>.
61yao commented on PR #9422:
URL: https://github.com/apache/pinot/pull/9422#issuecomment-1255538165
@walterddr friendly ping for review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org