Improved version of UniqueList: OrderedSet #76

lewispb · 2022-02-25T17:20:27Z

Closes #74

Using a benchmark script, we can see the improvement in performance by using a Sorted Set in some scenarios:

              user       system     total    real
ordered_set 0.131565   0.018613   0.150178 (  0.174106)
unique_list 1.109980   0.133186   1.243166 (  4.544614)

Initially, we can introduce this as a new datatype. But to follow on, a migration mechanism will be introduced so that we can replace unique_list.

A sorted set is optimal for this type of functionality because we can use fewer, more-performant Redis calls.

In order to ease the transition from UniqueList being backed by a Redis list, we can fallback to the legacy implementation for read operations. For write operations we first migrate to a sorted set, then retry.

lib/kredis/types/unique_list.rb

+      legacy_elements = legacy_list.elements
+      legacy_list.del
+      append(legacy_elements)
+    end


lewispb · 2022-02-25T17:32:09Z

lib/kredis/types/unique_list_legacy.rb

@@ -0,0 +1,29 @@
+# You'd normally call this a set, but Redis already has another data type for that
+class Kredis::Types::UniqueListLegacy < Kredis::Types::List


Renamed the existing UniqueList to UniqueListLegacy.

lib/kredis/types/unique_list.rb

+    end
+
+    def current_nanoseconds(negative:)
+      "%10.9f" % (negative ? -Time.now.to_f : Time.now.to_f)


lewispb · 2022-02-25T17:47:17Z

lib/kredis/types/unique_list.rb

+      end
+
+      multi do
+        zadd(elements_with_scores)


Consider LT / GT options for ordering safety, avoiding problems with clock skew.

Not possible due to Redis server compatibility target being version 4.

lib/kredis/types/unique_list.rb

+    end
+
+    def basis_score_components
+      [ redis_time + Time.now.utc.to_i, Time.now.utc.nsec ]


Use the Redis server time as a basis. Then add the current process time. Then add to that the index of the inserted elements as a microsecond component.

jeremy

Nice approach!

I'd introduce this as a distinct data type rather than migrating in place. Consider e.g. ordered_set which reflects the zset usage and scoring. Then, separately, introduce a pathway to auto-migrate from unique lists to ordered sets.

It'd be interesting to consider a migration abstraction for other types, too. Could be a proxy wrapper with declared fallback vs migrate behavior, e.g. migrates from: :OldType, to: NewType, fallback: [ methods ], migrate: [ methods ].

jeremy · 2022-03-11T23:45:35Z

lib/kredis/types/unique_list.rb

+    end
+
+    def process_start_time
+      @process_start_time ||= redis.time.join(".").to_f - process_uptime


I assume this may be used in replicated and clustered setups. Flagging just in case.

Do you envisage any problems if this were used in such a setup?

jeremy · 2022-03-11T23:46:28Z

lib/kredis/types/unique_list.rb

+    end
+
+    def process_uptime
+      Process.clock_gettime(Process::CLOCK_MONOTONIC)


Not sure about our spread of platform support, but this is limited to POSIX (so, probably not JRuby).

I think Process.clock_gettime is OK in JRuby 9 +

https://blog.dnsimple.com/2018/03/elapsed-time-with-ruby-the-right-way/

jeremy · 2022-03-11T23:49:37Z

lib/kredis/types/unique_list.rb

+    end
+
+    def trim(from_beginning:)
+      return unless limit&.positive?


Are negative/zero limits supported? If not, would fail earlier, e.g. in a #limit= method, and only check for nil limit here.

jeremy · 2022-03-11T23:58:50Z

lib/kredis/types/unique_list.rb

+      legacy_elements = legacy_list.elements
+      legacy_list.rename([ legacy_list.key, "_backup" ].join)
+      append(legacy_elements)
+    end


Does this need to be within an atomic MULTI…EXEC?

Like a UniqueList, but backed by a Redis sorted set rather than a list

* main: Bump version for 1.2.0 Use pipeline for migration too Ensure to pass the pipeline to super Use block parameter to pipeline in Redis#multi (rails#68) Note tested / supported versions of Redis in the Readme (rails#79) Return counter value after incrementing/decrementing Enum Bang setter (rails#82) Add reset to Cycle after_change callbacks Add example in Readme.md Support configuring custom keys via method invocation Use bundle add instead (rails#81)

lewispb · 2022-05-26T14:36:36Z

@jeremy thanks for all the feedback. This is much simpler now. I like the idea of splitting out the migration into a follow-up PR.

intrip

@lewispb Great approach! I've just left a note for your consideration

intrip · 2022-05-30T07:39:12Z

lib/kredis/types/ordered_set.rb

+      return if elements.empty?
+
+      elements_with_scores = types_to_strings(elements, typed).map.with_index do |element, index|
+        score = generate_base_score(negative: prepending) + index / 100000


Could you elaborate what index / 10000 is for?

Redis sorted sets use a double 64-bit floating point number to represent the score. In all the architectures we support, this is represented as an IEEE 754 floating point number

I was trying to store the maximum unique precision possible in Redis, to store scores (times). By dividing a large integer (seconds) by 100000 we get a float. e.g:

>> Time.now.to_i => 1653898149 >> Time.now.to_f / 100000 => 16538.98132595902 >> 3.times { p Time.now.to_f / 100000 } 16538.98262287975 16538.982622880398 16538.98262288052

Even though we lose some precision, each iteration produces a higher score than the previous one, perfect for our use case of appending.

intrip · 2022-05-30T07:54:08Z

lib/kredis/types/ordered_set.rb

+    def generate_base_score(negative:)
+      current_time = process_start_time + process_uptime
+
+      negative ? -current_time : current_time
+    end


Clever approach 👍!

Just a note for your consideration:

What would happen in case of skewed Redis clocks / concurrent writes? Is possible to face the scenario where appending 2 arrays would lead to mixing their elements?

# In process A ordered_set.append([1, 2, 3]) # In process B ordered_set.append([4, 5, 6]) # Is this possible? ordered_set.elements # => [1, 2, 4, 3, 5, 6]

Maybe a less clever approach that relies getting the max/min (with an optimistic lock) would be more resilient?

WATCH zset # get max element = ZRANGE zset -1 -1 # compute scores max +1 for each element MULTI # add elements ZADD zset elements # trim EXEC

neration

jeremy

Looking great.

Let's consider how to guide folks to the improved data structure. Perhaps remove the unique list example from the README, or move to the bottom and mark as deprecated? Could do this in a separate PR, since it also begs "well, how do I migrate?"

This reverts commit a0062d1.

* main: (21 commits) Bump version for 1.4.0 Update nokogiri for compatibility Revert "Improved version of UniqueList: OrderedSet (rails#76)" (rails#111) Add `last` to lists (rails#97) Improved version of UniqueList: OrderedSet (rails#76) Return Time objects instead of deprecated DateTime (rails#106) Fix possible deserialization of untrusted data Typecast return of Set#take (rails#105) Declare Active Model dependency (rails#107) Address LogSubscriber deprecation (rails#98) Account for time zones in DateTime serializations (rails#102) Add sample to set (rails#100) Bump version for 1.3.0 Allow Redis 5.x Add ltrim to lists Coalesce "current pipeline or redis" into the redis method itself Pefer a thread_mattr_accessor over a thread local variable Delete list of keys in batch (rails#90) Use a thread-local variable for pipeline Revert "Use block parameter to pipeline in Redis#multi (rails#68)" ...

…tialize * origin/main: (22 commits) Add kredis_ordered_set for OrderedSet usage in models Add a development console Bump version for 1.5.0 Fix ordered set prepend bug (rails#115) Unique list with sorted set (rails#114) Eliminating Ruby Warnings (rails#112) CI against Redis 7, Ruby 3.1, and Ruby 3.2 (rails#113) Bump version for 1.4.0 Update nokogiri for compatibility Revert "Improved version of UniqueList: OrderedSet (rails#76)" (rails#111) Add `last` to lists (rails#97) Improved version of UniqueList: OrderedSet (rails#76) Return Time objects instead of deprecated DateTime (rails#106) Fix possible deserialization of untrusted data Typecast return of Set#take (rails#105) Declare Active Model dependency (rails#107) Address LogSubscriber deprecation (rails#98) Account for time zones in DateTime serializations (rails#102) Add sample to set (rails#100) Bump version for 1.3.0 ...

lewispb added 7 commits February 25, 2022 14:59

Back UniqueList with a Redis Sorted Set

b326e1e

A sorted set is optimal for this type of functionality because we can use fewer, more-performant Redis calls.

Wrap unique list insert and trim in a transaction

649b59b

Fallback / migration mechanism for existing users of UniqueLists

f3c1a9e

In order to ease the transition from UniqueList being backed by a Redis list, we can fallback to the legacy implementation for read operations. For write operations we first migrate to a sorted set, then retry.

Update the Readme with sorted set backed unique list commands

0bf8ee9

Test remaining methods with migration fallbacks

3e8dad9

Be more specific about which errors to raise

ae53aed

Improve the fallback / migration system

c479e60

lewispb commented Feb 25, 2022

View reviewed changes

lib/kredis/types/unique_list.rb Outdated

legacy_elements = legacy_list.elements

legacy_list.del

append(legacy_elements)

end

This comment was marked as resolved.

Sign in to view

lewispb commented Feb 25, 2022

View reviewed changes

lib/kredis/types/unique_list.rb Outdated

end

def current_nanoseconds(negative:)

"%10.9f" % (negative ? -Time.now.to_f : Time.now.to_f)

This comment was marked as resolved.

Sign in to view

lewispb commented Feb 25, 2022

View reviewed changes

lewispb commented Feb 26, 2022

View reviewed changes

lib/kredis/types/unique_list.rb Outdated

end

def basis_score_components

[ redis_time + Time.now.utc.to_i, Time.now.utc.nsec ]

This comment was marked as resolved.

Sign in to view

dhh requested a review from jeremy February 26, 2022 09:33

lewispb added 2 commits March 1, 2022 10:34

Improve unique list scoring to avoid collisions

3773736

Use the Redis server time as a basis. Then add the current process time. Then add to that the index of the inserted elements as a microsecond component.

Update Readme with realistic Unique List scores

f28ff6d

lewispb force-pushed the unique_list_with_sorted_set branch from f7af6e0 to f28ff6d Compare March 1, 2022 10:39

jeremy reviewed Mar 12, 2022

View reviewed changes

lewispb added 8 commits May 26, 2022 15:14

Implement OrderedSet

71ba1a7

Like a UniqueList, but backed by a Redis sorted set rather than a list

Resolve pipeline deprecation warnings

1df0c66

Document OrderedSet

5cfdc28

Remove UniqueListLegacy

6035c06

Revert fallback options

f2f7940

Revert change to type_from

4beba5a

Enforce limit is not negative

316d9c6

Revert config proxying

1e62574

lewispb changed the title ~~Back UniqueList with a Sorted Set~~ Improved version of UniqueList: OrderedSet May 26, 2022

intrip reviewed May 30, 2022

View reviewed changes

Fix bug in score ge

cfe0310

neration

jeremy approved these changes May 31, 2022

View reviewed changes

dhh merged commit a0062d1 into rails:main Jun 18, 2023

dhh added a commit that referenced this pull request Jun 18, 2023

Revert "Improved version of UniqueList: OrderedSet (#76)"

c2ff13e

This reverts commit a0062d1.

dhh mentioned this pull request Jun 18, 2023

Revert "Improved version of UniqueList: OrderedSet" #111

Merged

dhh added a commit that referenced this pull request Jun 18, 2023

Revert "Improved version of UniqueList: OrderedSet (#76)" (#111)

1a96a74

This reverts commit a0062d1.

lewispb mentioned this pull request Jun 19, 2023

Unique list with sorted set #114

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved version of UniqueList: OrderedSet #76

Improved version of UniqueList: OrderedSet #76

lewispb commented Feb 25, 2022 •

edited

Loading

This comment was marked as resolved.

lewispb Feb 25, 2022

This comment was marked as resolved.

lewispb Feb 25, 2022

lewispb Feb 28, 2022

This comment was marked as resolved.

jeremy left a comment

jeremy Mar 11, 2022

lewispb May 26, 2022

jeremy Mar 11, 2022

lewispb May 26, 2022

jeremy Mar 11, 2022

lewispb May 26, 2022

jeremy Mar 11, 2022

lewispb commented May 26, 2022

intrip left a comment

intrip May 30, 2022

lewispb May 30, 2022

intrip May 30, 2022 •

edited

Loading

jeremy left a comment

		@@ -0,0 +1,29 @@
		# You'd normally call this a set, but Redis already has another data type for that
		class Kredis::Types::UniqueListLegacy < Kredis::Types::List

Improved version of UniqueList: OrderedSet #76

Improved version of UniqueList: OrderedSet #76

Conversation

lewispb commented Feb 25, 2022 • edited Loading

This comment was marked as resolved.

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

jeremy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lewispb commented May 26, 2022

intrip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intrip May 30, 2022 • edited Loading

Choose a reason for hiding this comment

jeremy left a comment

Choose a reason for hiding this comment

lewispb commented Feb 25, 2022 •

edited

Loading

intrip May 30, 2022 •

edited

Loading