[GitHub] [yunikorn-site] craigcondit commented on a diff in pull request #304: [YUNIKORN-1689] add preemption properties

["craigcondit (via GitHub)" <gi...@apache.org>]

craigcondit commented on code in PR #304:
URL: https://github.com/apache/yunikorn-site/pull/304#discussion_r1204621861


##########
docs/user_guide/queue_config.md:
##########
@@ -367,6 +367,26 @@ value (in other words, the priorities of tasks in the queue are ignored).
 
 See the documentation on [priority support](priorities.md) for more information.
 
+#### `preemption.policy`
+
+Supported values: `default`, `fence`, `disabled`
+
+Default value: `default`
+
+When using the `default` preemption policy, there is no limit preemption to the subtree.
+
+When using the [`fence` preemption policy](../design/preemption.md#preemption-fence), tasks running in or below the queue on which the property is set cannot preempt tasks outside the queue tree.
+
+When using the `disabled` preemption policy, tasks below the queue can't be victims.

Review Comment:
   Change "tasks below the queue" to "tasks running within the queue"



##########
docs/user_guide/queue_config.md:
##########
@@ -367,6 +367,26 @@ value (in other words, the priorities of tasks in the queue are ignored).
 
 See the documentation on [priority support](priorities.md) for more information.
 
+#### `preemption.policy`
+
+Supported values: `default`, `fence`, `disabled`
+
+Default value: `default`
+
+When using the `default` preemption policy, there is no limit preemption to the subtree.

Review Comment:
   Suggest: "When ..., preemption is enabled for the queue.



##########
docs/user_guide/queue_config.md:
##########
@@ -367,6 +367,26 @@ value (in other words, the priorities of tasks in the queue are ignored).
 
 See the documentation on [priority support](priorities.md) for more information.
 
+#### `preemption.policy`
+
+Supported values: `default`, `fence`, `disabled`
+
+Default value: `default`
+
+When using the `default` preemption policy, there is no limit preemption to the subtree.
+
+When using the [`fence` preemption policy](../design/preemption.md#preemption-fence), tasks running in or below the queue on which the property is set cannot preempt tasks outside the queue tree.
+
+When using the `disabled` preemption policy, tasks below the queue can't be victims.
+
+#### `preemption.delay`
+
+Supported values: any positive [Golang duration string](https://pkg.go.dev/time#ParseDuration)
+
+Default value: `30s`
+
+The property can only be set on a leaf queue. A queue with pending requests can only trigger the preemption after its resouice has been below its guaranteed resource for at least the duration.

Review Comment:
   This actually controls the minimum age of a task before preemption will be attempted. 
   
   Suggestion: "A queue with pending requests can only trigger preemption after it has been in the queue for at least this duration."



##########
docs/user_guide/queue_config.md:
##########
@@ -367,6 +367,26 @@ value (in other words, the priorities of tasks in the queue are ignored).
 
 See the documentation on [priority support](priorities.md) for more information.
 
+#### `preemption.policy`
+
+Supported values: `default`, `fence`, `disabled`
+
+Default value: `default`
+
+When using the `default` preemption policy, there is no limit preemption to the subtree.

Review Comment:
   Reason: Property is evaluated at each level of the queue tree (not inherited).



-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org