-
Notifications
You must be signed in to change notification settings - Fork 4
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
Address ambiguity for priority/score field names and additional priority score features #27
Comments
Well, if I have understood right, And sadly both |
Not exactly... A However, But you're correct, I think keeping only one in our API interface is better. I think everywhere we used If we revisit the concept, every priority queue has its own score (sometimes referred to as a priority value). I haven't searched much now to get the reference we used, but I remember seeing the description used in the book The Art of Multiprocessor Programming, Revised Reprint:
We initially built this project using Redis as the foundation, and their sorted sets always order elements based on the lowest value. This approach is common in other projects I've researched as well at the time. For instance, Netflix's launched Timestone after some months we created Deckard and they also assigns the highest priority to the element with the lowest score. What do you think? |
The idea of setting a priority during a ack/nack request seems fine to me. Regarding the inverse rationality between score and priority, this can be understandable, I think we should focus more on how we handle the score's boundaries, since this is what actually results in behavioral changes in the app. It can be done in 2 ways the way I see it and each one has its own caveat: Using 0 as a boundary:
Unbounded Score for positive and negative directions:I would not advise going into negative numbers, since it can lead to people mathematically manipulating them with subtractions in a wrong way, like adding a neg number resulting in a lower one. Which can lead to hard debugging or unexpected behaviors for the a given deckard use case but totally correct in mathematic terms. Even then, it could be done with all cares taken and it would allow one to virtually handle priorities draws infinitely on both directions, but I find it hard to be a necessary use case, so I would't back this up. |
Adressing the specific issue of the inverse logic between score X filter, I have always used the inverse interpretation, seems to be more academically accurate based on cientific definitions of a priority queue, as quoted by @lucasoares . |
Regrading the window filter request, seems ok and tottaly fine to me. |
Agreed. I think it's best to set the highest priority score as As stated in the Redis documentation, a sorted set score is:
And in more details:
We currently have the scores represented in Deckard as What do you think?
|
Got it.
Well remembered: the strict sequential ordering in a rank, without 'holes' between each element, would 'break expectations' in a large number of scenarios, indeed. @cezar-tech , I agree that most of the time lower number means higher priorities, but it's not universal (at least Scala and C++ use the higher-ordering - I have found this by looking at this article).
Any solution with consistent naming and behavior would do fine in my opinion: either using
I think the 'priority ordering' should be clearly stated in documentation (I didn't make a 'deep dive' in docs but I haven't seeing anything about this on wiki pages yet), to avoid any doubt for both users and contributors (for instance any replacement for Redis - as discussion #26 has started - should keep the same ordering convention). |
Fine by me then! Let's stick with Another question... If we let users set the score when doing an And we would only implement:
|
@lucasoares maybe just use one field for setting the absolute score value, no delta-like operations of add and subtracting, just let them set the score they want the message do have may yield more certain, less error-prone behavior and usability. I mean: just use a |
…ity manually (#29) For more details check the PR
Deckard currently assigns the default priority to the current timestamp when the message is added to the queue. While this works for most cases, some users may prefer to use their own priority algorithm.
To avoid breaking changes with protocol buffer default value, we should not allow users to set a
priority = 0
and should keep the current timestamp implementation for this case.Users will be able to set a negative priority if they want to.
Additionally, I will create new fields calledpriority_increase
andpriority_decrease
for ACK/NACK requests, as the currentscore_subtract
field is ambiguous. Increasing the priority means lowering its score (that's whyscore_subtract
have this name).We will also have two different fields for priority filtering on PULL requests. Currently, we have a
priority_filter
field and it is also ambiguous (currently filter messages to have a max score ofnow-score_subtract
) I suggest the creation of two new fields:max_score
andmin_score
:The
max_score
andmin_score
fields will be introduced as part of the priority filtering mechanism in the system. Here's an explanation of what they will mean:max_score
: sets the upper threshold for the priority score of a message to be returned in the pull request. Only messages with a priority score equal to or lower than themax_score
value will be returned.min_score
: sets the lower threshold for the priority score required for a message to be returned. Only messages with a priority score equal to or higher than themin_score
value will be returned.I'm not entirely sure if these are the best names for the fields because a lower priority score actually indicates a higher priority. Suggestions?
The text was updated successfully, but these errors were encountered: