⚡ Sangsakala Proxy 🏠 Home
Skip to content

Keep the LRU order of CallbackDataCache when persisting it - #5397

Open
waw666waw666 wants to merge 1 commit into
python-telegram-bot:masterfrom
waw666waw666:fix-callback-data-lru-order
Open

waw666waw666 wants to merge 1 commit into
python-telegram-bot:masterfrom
waw666waw666:fix-callback-data-lru-order

Conversation

@waw666waw666

Copy link
Copy Markdown

With arbitrary_callback_data and a persistence, the keyboards that are used the most can be the first ones evicted from CallbackDataCache, so their buttons start returning InvalidCallbackData.

persistence_data builds its list from self._keyboard_data.values(). For a cachetools.LRUCache, that goes through __getitem__ for every key, which marks each keyboard as just used, in insertion order. Application.update_persistence reads this property on every run, so after each run the LRU order is simply "oldest keyboard first", regardless of how recently it was pressed. The same order is used when loading the data after a restart.

cdc = CallbackDataCache(bot, maxsize=2)
menu = ...  # keyboard sent first, pressed regularly
old = ...   # keyboard sent later, never pressed again
# press a button of `menu`
cdc.persistence_data             # what update_persistence does
cdc.process_keyboard(new_markup)  # evicts `menu` instead of `old`

Changes proposed in this pull request:

  • Read the keyboards in persistence_data without updating the LRU order, and return them sorted by access_time (least recently used first).
  • Insert the keyboards sorted by access_time in load_persistence_data, so the order after a restart matches the one before (also for data stored by earlier versions).
  • Add a test that checks which keyboard is evicted after reading persistence_data, with and without reloading it.

Reading persistence_data went through LRUCache.__getitem__ for every
keyboard, which marked all keyboards as just used in insertion order.
Since Application.update_persistence reads it periodically, the most
used keyboards could become the first ones to be evicted. Loading the
data also inserted the keyboards in that order.

Read the keyboards without touching the LRU order, return them sorted
by access time and insert them in that order when loading.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant