-
Notifications
You must be signed in to change notification settings - Fork 102
Add Prioritized Priority Consumer Policy #965
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| namespace NATS.Client.JetStream.Models; | ||
|
|
||
| /// <summary> | ||
| /// The priority policy for consumer message selection. | ||
| /// </summary> | ||
| public enum ConsumerConfigPriorityPolicy | ||
| { | ||
| /// <summary> | ||
| /// No priority policy is set. | ||
| /// </summary> | ||
| None = 0, | ||
|
|
||
| /// <summary> | ||
| /// Messages are delivered based on priority level. | ||
| /// </summary> | ||
| Prioritized = 1, | ||
|
|
||
| /// <summary> | ||
| /// Messages overflow to the next available consumer. | ||
| /// </summary> | ||
| Overflow = 2, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -82,4 +82,17 @@ public record ConsumerGetnextRequest | |
| [System.Text.Json.Serialization.JsonPropertyName("id")] | ||
| [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)] | ||
| public string? Id { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Priority for message delivery when using prioritized priority policy. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Lower values indicate higher priority (0 is the highest priority). | ||
| /// Maximum priority value is 9. This field is only used when the consumer | ||
| /// has PriorityPolicy set to "prioritized". | ||
| /// </remarks> | ||
| [System.Text.Json.Serialization.JsonPropertyName("priority")] | ||
| [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)] | ||
| [System.ComponentModel.DataAnnotations.Range(0, 9)] | ||
| public byte Priority { get; set; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when these are serialized, are the names lowercased?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume None does not get written either. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,17 +263,6 @@ private async ValueTask<NatsJSConsumer> CreateOrUpdateConsumerInternalAsync( | |
| throw new NatsJSException("Cannot create consumers with multiple priority groups."); | ||
| } | ||
|
|
||
| if (config.PriorityPolicy is "pinned_client") | ||
| { | ||
| throw new NotImplementedException("Pinned clients are not supported yet."); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pinned_client wan't implemented anyway, so just removed it here. |
||
| } | ||
|
|
||
| // TODO: enum these values? | ||
| if (config.PriorityPolicy != null && config.PriorityPolicy != "none" && config.PriorityPolicy != "overflow" && config.PriorityPolicy != "pinned_client") | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't need this check any more. it's an enum now. |
||
| { | ||
| throw new NatsJSException("Cannot create consumers with priority policy other than 'overflow', 'pinned_client', or 'none'."); | ||
| } | ||
|
|
||
| var response = await JSRequestResponseAsync<ConsumerCreateRequest, ConsumerInfo>( | ||
| subject: subject, | ||
| new ConsumerCreateRequest | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,4 +252,39 @@ public void StreamConfigPersistMode_roundtrip_preserves_server_values() | |
| var jsonOut3 = Encoding.UTF8.GetString(bw3.WrittenSpan.ToArray()); | ||
| Assert.DoesNotContain("persist_mode", jsonOut3); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(ConsumerConfigPriorityPolicy.None, "\"priority_policy\":\"none\"")] | ||
| [InlineData(ConsumerConfigPriorityPolicy.Prioritized, "\"priority_policy\":\"prioritized\"")] | ||
| [InlineData(ConsumerConfigPriorityPolicy.Overflow, "\"priority_policy\":\"overflow\"")] | ||
| public void ConsumerConfigPriorityPolicy_test(ConsumerConfigPriorityPolicy value, string expected) | ||
| { | ||
| var serializer = NatsJSJsonSerializer<ConsumerConfig>.Default; | ||
|
|
||
| var bw = new NatsBufferWriter<byte>(); | ||
| serializer.Serialize(bw, new ConsumerConfig { PriorityPolicy = value }); | ||
|
|
||
| var json = Encoding.UTF8.GetString(bw.WrittenSpan.ToArray()); | ||
| Assert.Contains(expected, json); | ||
|
|
||
| var result = serializer.Deserialize(new ReadOnlySequence<byte>(bw.WrittenMemory)); | ||
| Assert.NotNull(result); | ||
| Assert.Equal(value, result.PriorityPolicy); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ConsumerConfigPriorityPolicy_null_test() | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we also have a test for |
||
| { | ||
| var serializer = NatsJSJsonSerializer<ConsumerConfig>.Default; | ||
|
|
||
| var bw = new NatsBufferWriter<byte>(); | ||
| serializer.Serialize(bw, new ConsumerConfig { PriorityPolicy = null }); | ||
|
|
||
| var json = Encoding.UTF8.GetString(bw.WrittenSpan.ToArray()); | ||
| Assert.DoesNotContain("priority_policy", json); | ||
|
|
||
| var result = serializer.Deserialize(new ReadOnlySequence<byte>(bw.WrittenMemory)); | ||
| Assert.NotNull(result); | ||
| Assert.Null(result.PriorityPolicy); | ||
| } | ||
| } | ||
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.
no need for documenting values. it's an enum now. it's a breaking change but since this is a fairly new feature, i don't expect many applications would be affected by it. required recompilation but the fix should be straight forward.
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.
Yeah, this is fine. I've done the same on brand new things.