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

Minor issues to be fixed #48

Open
2 tasks
panosfol opened this issue Feb 26, 2024 · 3 comments
Open
2 tasks

Minor issues to be fixed #48

panosfol opened this issue Feb 26, 2024 · 3 comments
Labels
good first issue Good for newcomers

Comments

@panosfol
Copy link
Collaborator

This issue keeps track of all the general issues that need attention in our codebase:

  • db.c:36 : we should not return if the connection fails in one of the databases.
  • postgres.c : there are malloc'd values that are not freed, should the connection with postgres fails.
    (This might lead to a bigger issue of keeping track of all the pointers that need to be freed in the various stages of the replication progress.)
@panosfol panosfol added the good first issue Good for newcomers label Feb 26, 2024
@charmitro
Copy link
Contributor

charmitro commented Feb 26, 2024

[ ] db.c:36 : we should not return if the connection fails in one of the databases.

Not sure about that...

Let's think this scenario

A user has 2 databases in his config file, a postgres and a Mongo (assuming we support it) and first in operation is Postgres, which fails for whatever reason, does he want to continue with Mongo replication?


[ ] postgres.c : there are malloc'd values that are not freed, should the connection with postgres fails.
(This might lead to a bigger issue of keeping track of all the pointers that need to be freed in the various stages of the replication progress.)

I'll investigate and return to this later.

@panosfol
Copy link
Collaborator Author

[ ] db.c:36 : we should not return if the connection fails in one of the databases.

Not sure about that...

Let's think this scenario

A user has 2 databases in his config file, a postgres and a Mongo (assuming we support it) and first in operation is Postgres, which fails for whatever reason, does he want to continue with Mongo replication?

[ ] postgres.c : there are malloc'd values that are not freed, should the connection with postgres fails.
(This might lead to a bigger issue of keeping track of all the pointers that need to be freed in the various stages of the replication progress.)

I'll investigate and return to this later.

Yes i believe he should continue with Mongo replication. If a user tries to replicate a lot of databases
and he made a mistake in the configuration, shouldn't he just 'lose' replication process on only one?
There is some time wasted for him if all failed right?

@charmitro
Copy link
Contributor

Yes i believe he should continue with Mongo replication. If a user tries to replicate a lot of databases and he made a mistake in the configuration, shouldn't he just 'lose' replication process on only one? There is some time wasted for him if all failed right?

I agree but I also believe that users should be able to disable this. Let's leave it as is for now (since we support only one database) and we change it to not return later, plus a quit_on_error config field under [general].

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

2 participants