-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IGNITE-23601 Create broadcast partitioned compute method #4871
base: main
Are you sure you want to change the base?
Conversation
97f6cba
to
35a8ddd
Compare
35a8ddd
to
a658c59
Compare
* | ||
* @return list of partitions numbers associated with this job. | ||
*/ | ||
@Nullable List<Integer> partitions(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nullable List<Integer> partitions(); | |
@Nullable List<org.apache.ignite.table.partition.Partition> partitions(); |
Should be compatible with PartitionManager
API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the type to Partition
) { | ||
Objects.requireNonNull(tableName); | ||
Objects.requireNonNull(descriptor); | ||
return failedFuture(new UnsupportedOperationException("Not implemented")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please create a ticket and add TODO for client impl. Ideally also tickets for other clients (.NET, C++).
@@ -244,6 +244,98 @@ default <T, R> Map<ClusterNode, R> executeBroadcast( | |||
return map; | |||
} | |||
|
|||
/** | |||
* Submits a {@link ComputeJob} of the given class for an execution on nodes where the primary replicas of a partitions of the specified | |||
* table are located. The partition indices are passed to the job in the {@link JobExecutionContext#partitions()}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any "partition pinning"? What if the partition is moved to another node while the job is executing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use "best-effort" approach. If partition is moved, then it will not be local, that's it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the use case? What is the difference from a regular broadcast call, apart from access to the partition list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically it's a shorthand version of the broadcast with manually getting the table, getting the partition list and filtering it to get the partitions on a local node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then, in my opinion, we don't need those new broadcast methods. Instead let's simplify "get local partitions for a table" scenario by extending PartitionManager
API.
Thoughts?
Add more tests for the new methods
https://issues.apache.org/jira/browse/IGNITE-23601
This adds new compute API methods for executing jobs based on the table partitioning. These methods take a table name and submit jobs on nodes when the primary replica for the specified table is located, passing the list of partition indices on this node via the
JobExecutionContext
.