mingmwang commented on code in PR #261:
URL: https://github.com/apache/arrow-ballista/pull/261#discussion_r984160816
##########
ballista/rust/core/proto/ballista.proto:
##########
@@ -521,12 +554,12 @@ message FetchPartition {
uint32 port = 6;
}
-// Mapping from partition id to executor id
message PartitionLocation {
- PartitionId partition_id = 1;
- ExecutorMetadata executor_meta = 2;
- PartitionStats partition_stats = 3;
- string path = 4;
+ uint32 map_partition_id = 1;
+ PartitionId partition_id = 2;
Review Comment:
Sure, will add more comments in the code. Here the new added
`map_partition_id` is the partition_id of the map stage who produce this
shuffle data. But the original `PartitionId partition_id` has different
meanings in different places.
Sometimes it stands for a task_id, and here it stands for a shuffle
partition id(composition of map_stage_id + reduce partition id + job_id), a
mixed up of map and reduce infos together.
So if we do not consider the backward compatibility, I would suggest to
unnest this and make the PartitionLocation a plan struct like blow:
````
message PartitionLocation {
string job_id = 1;
uint32 map_stage_id = 2;
uint32 map_partition_id = 3;
uint32 partition_id = 4;
ExecutorMetadata executor_meta = 5;
PartitionStats partition_stats = 6;
string path = 7;
}
````
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
