Re-implement looking up multiple messages with more flexibility.#9
Re-implement looking up multiple messages with more flexibility.#9
Conversation
Allow mixing results across multiple topics, with potentially differing access levels. Allow reversing the order of results. Use more efficient paging.
jnation3406
left a comment
jnation3406
left a comment
There was a problem hiding this comment.
Looks okay to me. One thing I have noticed is that if you have a topic filter (one or more), then while the initial query is fairly fast (1-2s), each subsequent page load with pagination takes much longer (10-12s). This same slowdown for pagination doesn't seem to appear when no topic filters are present.
| if offset > 0: | ||
| offset -= 1 | ||
| topics = set() | ||
| for record in self.data.values(): |
There was a problem hiding this comment.
Like your deleted comment above suggests, this is very inneficient right? Maybe it would be better to cache public topics (and update cached topics on new message ingestion)?
There was a problem hiding this comment.
oh is this just a test fixture - then ignore my comment i guess
There was a problem hiding this comment.
Right, this code path should only be used by tests.
| else: | ||
| full_clause = None | ||
| if pub_clause is not None and full_clause is not None: | ||
| q = q.where(sqlalchemy.or_(pub_clause, full_clause)) |
There was a problem hiding this comment.
Is this equally as efficient as combining the topics into one big list and doing a single where topic in clause?
There was a problem hiding this comment.
It's not as efficient, but is required for correctness because the 'public' clause includes different restrictions (topic is one of the public topics, and the message is public). We must not lose that restriction on things with 'public' access level, and don't want to enforce it on things with 'full' access level.
That is not supposed to happen, and I don't think I've seen anything like it in my testing. I'll try to check it again, but do you have any representative API requests for which this occurs? |
|
After investigation of the perfromance issue, mitigation is possible using an additional meta-data table in the database, which will be added in the next PR. |
2 participants
Allow mixing results across multiple topics, with potentially differing access levels.
Allow reversing the order of results.
Use more efficient paging.
This is the first phase of updates to support a better REST API/Web UI, which includes all changes that could be made without changes to the data stored in the database.