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

Supporting multiple same-system databases #63

Open
panosfol opened this issue Apr 11, 2024 · 3 comments · May be fixed by #73
Open

Supporting multiple same-system databases #63

panosfol opened this issue Apr 11, 2024 · 3 comments · May be fixed by #73
Labels
config enhancement New feature or request help wanted Extra attention is needed

Comments

@panosfol
Copy link
Collaborator

Currently the way we parse the config file, it doesn't support multiple instances of the same database system.
The user can't provide multiple postgres databases for example to be replicated, because the sections will have overlap
and the previous database information will be overwritten.

Maybe one possible solution is to save each database information on an array. For example"

typedef struct config_t {
	postgres_t *postgres_config;
	smtp_t *smtp_config;
	general_t *general_config;
} config_t;

instead of this, we implement it like this:

typedef struct config_t {
	postgres_array_t **postgres_config;
	smtp_t *smtp_config;
	general_t *general_config;
} config_t;

The size of the array could be malloc'ed at runtime after we parse the .ini file. This of course will require some
refactoring from our side (on db.c for example), therefore I believe should be given high priority to avoid refactoring
larger parts of the codebase in the future.

@panosfol panosfol added enhancement New feature or request help wanted Extra attention is needed config labels Apr 11, 2024
@charmitro
Copy link
Contributor

charmitro commented Apr 12, 2024

Let's discuss the following to determine if this will work


Using a linked list provides more flexibility compared to a fixed-size array, as the number of database instances doesn't need to be known in advance. Each new instance is dynamically allocated and added to the list.


Modify the config_t struct to use a linked list for each database type instead of a single struct. Define a new struct (e.g., postgres_node_t) to represent a node in the linked list, containing the database configuration and a pointer to the next node.

NOT SURE ABOUT THIS: Update the handler function in config.c to dynamically allocate a new node for each database instance encountered in the INI file. When a new section (e.g., [postgres]) is found, create a new node, populate its configuration from the INI values, and prepend it to the linked list.

Refactor the execute_db_operations function in db.c to traverse the linked list for each database type. For each node, create the corresponding database operations struct and add it to the available_dbs array.

Update the free_config function to properly deallocate the linked list and its associated memory. Traverse the linked list for each database type, freeing the configuration fields and the node itself.

Review and update other parts of the codebase that interact with the database configurations to ensure compatibility with the new linked list structure. This may include functions in postgres.c, command-line parsing, error handling, etc.

This change requires careful refactoring of several parts of the codebase.

Let's start this incrementally,

  • configuration parsing and data structures
  • updating the logic in other files to utilize the new structures.
  • It is crucial to add test cases along the way if we can...

@panosfol
Copy link
Collaborator Author

After analyzing this i believe it is doable and a well-thought-out solution. Will a seperate issue be created, outlining each task? Or should I start implementing this right away according to the tasks posted here?

@charmitro
Copy link
Contributor

After analyzing this i believe it is doable and a well-thought-out solution. Will a seperate issue be created, outlining each task? Or should I start implementing this right away according to the tasks posted here?

Evaluate the existing design and provide a detailed one with additions you found that are needed.

@panosfol panosfol linked a pull request Jul 5, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants