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

Backend fails to start after removing Redis #2685

Closed
TueeNguyen opened this issue Jan 20, 2022 · 9 comments
Closed

Backend fails to start after removing Redis #2685

TueeNguyen opened this issue Jan 20, 2022 · 9 comments
Assignees
Labels
area: back-end area: redis Redis Database related type: bug Something isn't working
Milestone

Comments

@TueeNguyen
Copy link
Contributor

TueeNguyen commented Jan 20, 2022

What happened:

150237254-42c81164-7da3-408f-8910-ef4a9b95765a

PR #2615 is likely to have caused the bug

What should have happened:

Containers should have built and run successfully

How to reproduce:

This bug occurs when you run pnpm start with your Redis database being fresh, not populated. It will run normally if you have run the project before.

@TueeNguyen TueeNguyen added type: bug Something isn't working area: redis Redis Database related area: back-end labels Jan 20, 2022
@TDDR TDDR added this to the 2.6 Release milestone Jan 20, 2022
@TueeNguyen
Copy link
Contributor Author

TueeNguyen commented Jan 20, 2022

After talking to @HyperTHD, I think the problem only occurs in development.

satellite returns redis connection to redis server with hostname 'redis' here, but the server with that hostname doesn't exist. The redis server in the docker container has 127.0.0.1 as hostname. Below is the result when I run redis-cli in the container

image

In development.yml, redis server probably falls back to 127:0:0:1 as it hostname

redis:
ports:
- '6379:6379'

@TueeNguyen
Copy link
Contributor Author

TueeNguyen commented Jan 20, 2022

The possible fixes for this:

  1. I added an environment variable in development.yml
redis:
    environment:
      - REDIS_URL=reids://127.0.0.1:6379
    ports:
      - '6379:6379'
  1. Or uncomment REDIS variables in env.development, not sure why these are commented?
REDIS_URL=redis://127.0.0.1
REDIS_PORT=6379

Though, I need some help to make sure that it runs normally on production/staging

@humphd
Copy link
Contributor

humphd commented Jan 20, 2022

This is an interesting problem. I'm tempted to say that the fix for it is to leave the backend how it is now, and not bother updating it to use Satellite.

The reason for this is that the code in the backend already does the right thing if the REDIS_URL and REDIS_PORT aren't defined:

parseUrl(process.env.REDIS_URL, process.env.REDIS_PORT) || 'redis://127.0.0.1:6379';

If we use Satellite, we have to define a REDIS_URL, and the only way to do this is to pass in an env file. We used to do this with the backend, but I don't think we do now (@manekenpix am I right about that?). So there's no easy way to pass this into the backend.

We get around this in development by re-using the config from config/env.development here

if (process.env.NODE_ENV !== 'production') {
; but that doesn't work in production, which is likely why we crashed.

@HyperTHD
Copy link
Contributor

Thing is, in staging/production, we've redefined the redis_url to point to the container itself, which is defined as follows:

https://github.com/Seneca-CDOT/telescope/blob/master/docker/production.yml#L41
https://github.com/Seneca-CDOT/telescope/blob/master/docker/production.yml#L177

I tried to connect to redis this way with dev while talking with @TueeNguyen to no avail, with the only way being to do so the way he mentioned above.

It's a sticky situation, the way the backend does it is fine, and honestly once the parser service gets added, we shouldn't worry about it since the backend's going to be removed. The service that uses redis, the posts service, grabs it from Satellite just fine as well

@TueeNguyen
Copy link
Contributor Author

I'm ok with leaving backend the way it is, it's not sth we prioritize

@humphd
Copy link
Contributor

humphd commented Jan 20, 2022

I think we should focus on the parser service and fix this by removing the backend completely. Then we don't have multiple hacks in place to make this work.

@TueeNguyen you cool if we close this?

@TueeNguyen
Copy link
Contributor Author

Yes!

@humphd
Copy link
Contributor

humphd commented Jan 20, 2022

Thanks for doing the research and digging into this. Even though we didn't go with this solution, having the options in front of us was really important for being able to move forward.

@TueeNguyen
Copy link
Contributor Author

TueeNguyen commented Jan 21, 2022

I believe someone will dig this one up for his issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: back-end area: redis Redis Database related type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants