Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expiry for Links #22

Open
ChakshuGautam opened this issue Jun 1, 2022 · 29 comments
Open

Expiry for Links #22

ChakshuGautam opened this issue Jun 1, 2022 · 29 comments
Labels
good first issue Good for newcomers

Comments

@ChakshuGautam
Copy link
Contributor

ChakshuGautam commented Jun 1, 2022

Is your feature request related to a problem? Please describe.
Currently the links are persisted indefinitely. We should remove redirection and allow hashes to get reused once the link expires.

Describe the solution you'd like
A link should have the following states

  1. draft - no hash assigned
  2. live
  3. expired - hash can be repurposed
@ChakshuGautam ChakshuGautam added the good first issue Good for newcomers label Jun 1, 2022
@Gurdeep475
Copy link

I would like to work on this issue.

@ChakshuGautam
Copy link
Contributor Author

Go for it !!!

@psankhe28
Copy link

Is this issue solved? @ChakshuGautam

@ChakshuGautam
Copy link
Contributor Author

Not really. Do you want to pick this up?

@psankhe28
Copy link

Yeah. I needed some more details regarding the exact requirement.

@ChakshuGautam
Copy link
Contributor Author

Can you share an approach how you want to implement this?

@psankhe28
Copy link

Actually, I haven't got a clear idea about what exactly is needed to be done

@ChakshuGautam
Copy link
Contributor Author

The project is a URL shortener that stores link indefinitely. For some campaigns, the users don't need the link to work indefinitely. It should show "link expired" after expiry.

@psankhe28
Copy link

Can you list some of the campaigns if possible?

@ChakshuGautam
Copy link
Contributor Author

ChakshuGautam commented May 5, 2023

It's a simple model right now. You can see the schema here -

We need a new field called status here that will be an enum with the above values and an expiry field that will hold the expiry date of a link. The system needs to check when the current date is more than expiry and stop resolving the short link to the long link and send a 410.

@psankhe28
Copy link

As per my understanding,
Basically, the expiry field will hold the expiry date of the link. So the requirement is to add these fields and implement the checking function.
Is there anything else that I missed?

@ChakshuGautam
Copy link
Contributor Author

Yes, this is it. Optionally we can have a custom 410 page as well that can come up. That would be a delight functionality. You can pick a design.

@psankhe28
Copy link

Okay. Thanks for helping me out and solving my queries. I will work on this and create a pr soon.

@psankhe28
Copy link

psankhe28 commented May 8, 2023

I had a doubt @ChakshuGautam where should the checking function be implemented? In the same file? Also, where can I create the 404 page? Can you please specify the location for that too

@psankhe28
Copy link

Could you please answer my above question? @ChakshuGautam

@ChakshuGautam
Copy link
Contributor Author

Can you share your progress till now? What have you tried, what worked what didn't?

If you already have an opinion on this can you share that as well?

@psankhe28
Copy link

I have added a new field called status which is an enum with the values mentioned and an expiry field that will hold the expiry date of a link. For the function where the system needs to check when the current date is more than expiry and stop resolving the short link to the long link and send a 410, I am unable to understand where should I like the logic for it and also where should I create the design for the 410 page

@ChakshuGautam
Copy link
Contributor Author

You might want to change this place -

return res.redirect(302, reRouteURL);
. If the link is expired don't return a 302.

@psankhe28
Copy link

Thank you. I will work on this and create a pr soon.

@kapiswayprakhar15
Copy link

Is this issue resolved sir @ChakshuGautam ?

@ChakshuGautam
Copy link
Contributor Author

Not yet. Please go ahead and raise a PR.

@kapiswayprakhar15
Copy link

Okay , as soon as I finish this, I'll raise a PR.

@Palaksharma23
Copy link

Hi @ChakshuGautam, is the issue resolved? I would like to work on this issue.

@ChakshuGautam
Copy link
Contributor Author

Not yet @Palaksharma23. This is still open.

@Nazi-pikachu
Copy link
Collaborator

Nazi-pikachu commented Jul 20, 2023

I have been thinking about Possible solution to manage expired links and here is a crisp summary of possible solution after discussing with @yuvrajsab

  1. Query from DB : First and straightforward solution is to remove check the link for expiry each time user click and then conditionally send the response.
  2. Use some in-memory DB to make the search of the links faster : To optimize the previous method we can use Redis to store the linkId and expiry date which can speed up the search but this would take up additional memory in Redis.
  3. Set up a cron job at every interval but we have to tradeoff some precision in expiration time : If we can afford to tradeoff accuracy of expiration time then we can setup a cron job which clean expired links in DB in a preiodical manner.
    In all the above approaches we have to tradeoff something for other. Can you please share you opinion on what would be the best way to go forward @ChakshuGautam

@yuvrajsab
Copy link
Member

hey @Nazi-pikachu, I think there is one more possible solution to this problem; we can leverage Redis here. we can add a Redis layer on top of Postgres. when a user creates a new link it will get added to the Redis with an expiry. so that the link will get automatically popped out of the cache and will not be available anymore. And we can create a cronjob that will sync the Postgres with the Redis (meaning disable/delete the links which are not present in the cache). But you'll need to handle all the edge cases properly. lemme know what you think about this.

@Nazi-pikachu
Copy link
Collaborator

Hey @yuvrajsab
I think this would be a good way of doing it.
As I was thinking about what we should prioritize i stumble upon the vision statement of yaus: Batteries included URL Shortener that is designed for speed and scalability.
This gave me a strong reason of going with the redis as this would be the most performant way to handle it.
Let me know your thoughts and i will start with the implementation.

@yuvrajsab
Copy link
Member

@Nazi-pikachu, I too think this is the best idea out of all we considered. You can identify all the places where we can involve cache and it could be possible that we could lose cache data(or key) unexpectedly; for that, we need to keep the cache synced with db meaning delete from the cache if something is deleted from db or add to the cache if something is added to the db. Remember all the side-effects should be done through RabbitMQ. As of now, I'm not seeing any drawback to this approach You can start implementing this👍.

@Nazi-pikachu
Copy link
Collaborator

Nazi-pikachu commented Jul 30, 2023

Hey @yuvrajsab
To save the links directly to redis we need hashid which in the curernt setting is being set and return by DB.
I was thinking to make the write operations async using RMQ but now i think this won't be possible until and unless we generate the hashid in service itself and then persist it in the DB.
Also why do we need to add the click-count to redis and then sync it with the DB ? Why cant we push it to the RMQ and then directly into the DB. Do we need to have a highly accurate click-count? Can't we trade off the accuracy for a less error prone solution.
Can you give your suggestions on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants