make further requested changes, optimize for ordered_set table times

guides/advanced/pluggable_caching.md

@@ -10,7 +10,7 @@ this, implement the behaviours exported by the cache modules under
 > The exception to the above is the `Nostrum.Cache.MessageCache`, which does not
 > include an ETS-based implementation, and defaults to a NoOp cache. This is
 > an intentional design decision because caching messages consumes a
-> lot more memory than other objects, and are often not needed by most users.
+> lot more memory than other objects, and is often not needed by most users.
 
 Use the `[:nostrum, :caches]` configuration for configuring which cache
 implementation you want to use. This can only be set at dependency compilation

lib/nostrum/cache/message_cache.ex

@@ -95,10 +95,9 @@ defmodule Nostrum.Cache.MessageCache do
   match specifications in your `TraverseFun` and implement a `LookupFun` as
   documented.
 
-  The query handle must return items in the form `{channel_id, author_id, message}`, where:
-  - `channel_id` is a `t:Nostrum.Struct.Channel.id/0`,
-  - `author_id` is a `t:Nostrum.Struct.User.id/0`, and
-  - `message` is a `t:Nostrum.Struct.Message.t/0`.
+  The query handle must return items in the form `{message_id, message}`, where:
+  - `message_id` is a `t:Nostrum.Struct.Message.id/0`
+  - `message` is a `t:Nostrum.Struct.Message.t/0`
 
   If your cache needs some form of setup or teardown for QLC queries (such as
   opening connections), see `c:wrap_qlc/1`.
@@ -130,7 +129,7 @@ defmodule Nostrum.Cache.MessageCache do
   Used to constrain the return values of functions that can return
   a list of messages from the cache.
   """
-  @type timestamp_like :: integer() | DateTime.t()
+  @type timestamp_like() :: Snowflake.t() | DateTime.t()
 
   # User-facing
 

lib/nostrum/cache/message_cache/mnesia.ex

@@ -5,22 +5,27 @@ if Code.ensure_loaded?(:mnesia) do
 
     #{Nostrum.Cache.Base.mnesia_note()}
 
-    By default, the cache will store up to 10,000 messages,
-    and will evict the 100 oldest messages when the limit is reached.
+    By default, the cache will store up to `10_000` messages,
+    and will evict the `100` oldest messages when the limit is reached.
 
     The reason for the eviction count is that with mnesia it is more efficient to
     find X oldest records and delete them all at once than to find the oldest
     record and delete it each time a new record is added.
 
     The Mnesia cache supports the following configuration options:
     - `size_limit`: The maximum number of messages to store in the cache.
-    default: 10,000
+    default: `10_000`
     - `eviction_count`: The number of messages to evict when the cache is full.
-    default: 100
+    default: `100`
     - `table_name`: The name of the Mnesia table to use for the cache.
     default: `:nostrum_messages`
     - `compressed`: Whether to use compressed in memory storage for the table.
-    default: false
+    default: `false`
+    - `type`: Sets the type of Mnesia table created to cache messages.
+    Can be either `:set` or `:ordered_set`, by choosing `:ordered_set` the
+    eviction of the oldest messages will be more efficient, however it means
+    that the table cannot be changed to only store its contents on disk later.
+    default: `:ordered_set`
 
     To change this configuration, you can add the following to your
     `config.exs`:
@@ -31,7 +36,7 @@ if Code.ensure_loaded?(:mnesia) do
         messages: {Nostrum.Cache.MessageCache.Mnesia,
                    size_limit: 1000, eviction_count: 50,
                    table_name: :my_custom_messages_table_name,
-                   compressed: true}
+                   compressed: true, type: :set}
       }
     ```
 
@@ -49,6 +54,7 @@ if Code.ensure_loaded?(:mnesia) do
     @maximum_size @config[:size_limit] || 10_000
     @eviction_count @config[:eviction_count] || 100
     @compressed_table @config[:compressed] || false
+    @table_type @config[:type] || :ordered_set
 
     @behaviour Nostrum.Cache.MessageCache
 
@@ -67,19 +73,7 @@ if Code.ensure_loaded?(:mnesia) do
     @impl Supervisor
     @doc "Set up the cache's Mnesia table."
     def init(_init_arg) do
-      ets_props =
-        if @compressed_table do
-          [:compressed]
-        else
-          []
-        end
-
-      options = [
-        attributes: [:message_id, :channel_id, :author_id, :data],
-        index: [:channel_id, :author_id],
-        record_name: @record_name,
-        storage_properties: [ets: ets_props]
-      ]
+      options = table_create_attributes()
 
       case :mnesia.create_table(@table_name, options) do
         {:atomic, :ok} -> :ok
@@ -238,18 +232,58 @@ if Code.ensure_loaded?(:mnesia) do
       :mnesia.activity(:sync_transaction, fun)
     end
 
+    @doc false
+    def table_create_attributes do
+      ets_props =
+        if @compressed_table do
+          [:compressed]
+        else
+          []
+        end
+
+      [
+        attributes: [:message_id, :channel_id, :author_id, :data],
+        index: [:channel_id, :author_id],
+        record_name: @record_name,
+        storage_properties: [ets: ets_props],
+        type: @table_type
+      ]
+    end
+
     # assumes its called from within a transaction
     defp maybe_evict_records do
       size = :mnesia.table_info(@table_name, :size)
 
       if size >= @maximum_size do
-        oldest_message_ids =
-          :nostrum_message_cache_qlc.sorted_by_age_with_limit(__MODULE__, @eviction_count)
+        case :mnesia.table_info(@table_name, :type) do
+          :set ->
+            evict_set_records()
 
-        Enum.each(oldest_message_ids, fn message_id ->
-          :mnesia.delete(@table_name, message_id, :write)
-        end)
+          :ordered_set ->
+            evict_ordered_set_records()
+        end
       end
     end
+
+    defp evict_set_records do
+      oldest_message_ids =
+        :nostrum_message_cache_qlc.sorted_by_age_with_limit(__MODULE__, @eviction_count)
+
+      Enum.each(oldest_message_ids, fn message_id ->
+        :mnesia.delete(@table_name, message_id, :write)
+      end)
+    end
+
+    defp evict_ordered_set_records do
+      first = :mnesia.first(@table_name)
+
+      Enum.reduce(1..(@eviction_count - 1), [first], fn _i, [key | _rest] = list ->
+        next_key = :mnesia.next(@table_name, key)
+        [next_key | list]
+      end)
+      |> Enum.each(fn key ->
+        :mnesia.delete(@table_name, key, :write)
+      end)
+    end
   end
 end

test/nostrum/cache/message_cache/mnesia_additional_test.exs

@@ -83,6 +83,27 @@ defmodule Nostrum.Cache.MessageCache.MnesiaAdditionalTest do
         assert {:ok, %Message{id: ^id}} = MessageCache.Mnesia.get(id)
       end
     end
+
+    test "eviction for tables of type set works as well" do
+      # drop and recreate the table with a different type
+      MessageCache.Mnesia.teardown()
+
+      table_create_attributes =
+        MessageCache.Mnesia.table_create_attributes()
+        |> Keyword.put(:type, :set)
+
+      {:atomic, :ok} = :mnesia.create_table(MessageCache.Mnesia.table(), table_create_attributes)
+
+      for id <- 1..11, do: MessageCache.Mnesia.create(Map.put(@test_message, :id, id))
+
+      for id <- 1..4 do
+        assert MessageCache.Mnesia.get(id) == {:error, :not_found}
+      end
+
+      for id <- 5..11 do
+        assert {:ok, %Message{id: ^id}} = MessageCache.Mnesia.get(id)
+      end
+    end
   end
 
   describe "update/1" do