AscendingCreations / AxumSession

Axum Session Management Libraries that use Sqlx
MIT License
136 stars 28 forks source link

delete_by_expiry differs for databases #50

Closed alexichepura closed 1 year ago

alexichepura commented 1 year ago

Hello, I was updating my pool to newer axum_session version https://github.com/alexichepura/lapa/blob/main/admin/src/axum_session_prisma.rs

And noticed delete_by_expiry differs for databases.

mysql uses ~greater~ less than (i think this is correct) (UPDATED LINE) https://github.com/AscendingCreations/AxumSessions/blob/main/src/databases/mysql.rs#L46

postgres and sqlite uses ~less~ greater than (i think this is bug) (UPDATED LINE) https://github.com/AscendingCreations/AxumSessions/blob/main/src/databases/postgres.rs#L46 https://github.com/AscendingCreations/AxumSessions/blob/main/src/databases/sqlite.rs#L46

These bugs are not leading to issue yet because real deletion goes later with correct sign. But anyway it's confusing.

And maybe can be changed to deletion of loaded already ids: ... WHERE id IN (value1, value2, ...);

genusistimelord commented 1 year ago

So, memory_lifespan is what determines removal of already loaded Data from memory and it is the cycle on which the expiration within memory occurs. expiration_update is used to run the database deletion cycle which then runs the delete_by_expiry. The Delete By expiry Returns a Vec of Deleted ID's for the new key_store feature which uses a fastbloom table to store all currently in use keys to speed up key creation on heavy websites. This ensures that all the id's that got removed are removed from the table to reduce the likely hood of a false positive's within the table.

And maybe can be changed to deletion of loaded already ids: ... WHERE id IN (value1, value2, ...);

This really would not really increase speed and would cause more issues as we would need to keep everything within memory to know if we should remove it from the database or not. The database is used to reduce Ram usage of the sessions and to ensure longer term storage of the sessions for later use based upon the Sessions Cookie expiration and database lifetimes. They are also there to guarantee that if the Server crashed the old sessions still exist upon restart.

These bugs are not leading to issue yet because real deletion goes later with correct sign.

This was 100% an oversight on my part. I am pushing fixes to fix this issue.

alexichepura commented 1 year ago

Oh, i mixed wording on greater than and less than.. Yep, sorry for misleading on that.

Thank you for these great libs!

genusistimelord commented 1 year ago

No problem. I am also updating the iter() part to remove the clone of the String which is not needed. that should speed that process up a lot more.

genusistimelord commented 1 year ago

I am publishing a new release with these fixes. Should be up within an hour.