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

Split struct prometheus from struct configuration #155

Closed
wants to merge 1 commit into from

Conversation

An-DJ
Copy link
Contributor

@An-DJ An-DJ commented Jun 29, 2021

Split struct prometheus from struct configuration.

Close #153

@@ -142,6 +142,11 @@ extern void* shmem;
*/
extern void* pipeline_shmem;

/**
* The shared memory segment for prometheus connection
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"prometheus connection" -> "the Prometheus data"

*/
struct prometheus_connection
{
char database[MAX_DATABASE_LENGTH]; /**< The database */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

struct prometheus_connection will have a 1-to-1 relationship with struct connection, so this can be an empty struct to begin with

@@ -235,6 +250,7 @@ struct prometheus

atomic_ulong server_error[NUMBER_OF_SERVERS]; /**< The number of errors for a server */
atomic_ulong failed_servers; /**< The number of failed servers */
struct prometheus_connection prometheus_connections[MAX_NUMBER_OF_CONNECTIONS]; /**< The number of prometheus connections */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be a FMA field, so we only use the memory that we need.

See how struct connection[]; is initialized after max_connections is known.


for (int i = 0; i < config->max_connections; i++)
{
memset(prometheus->prometheus_connections[i].database, 0, MAX_DATABASE_LENGTH);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this memset with a general memset(&prometheus->prometheus_connections[i], 0, sizeof(struct prometheus_connection);


for (int i = 0; i < NUMBER_OF_SERVERS; i++)
{
atomic_store(&config->prometheus.server_error[i], 0);
atomic_store(&prometheus->server_error[i], 0);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to reset the prometheus_connections as well

@@ -795,6 +845,10 @@ general_information(int client_fd)

config = (struct configuration*)shmem;

struct prometheus* prometheus;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep definitions at the start of the function

src/main.c Outdated
@@ -396,6 +398,19 @@ main(int argc, char **argv)
pgagroal_init_configuration(shmem);
config = (struct configuration*)shmem;

prometheus_shmem_size = sizeof(struct prometheus);
if (pgagroal_create_shared_memory(prometheus_shmem_size, HUGEPAGE_OFF, &prometheus_shmem))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huge page settings needs to come from the configuration

@jesperpedersen jesperpedersen added the enhancement Improvement to an existing feature label Jun 29, 2021
@jesperpedersen
Copy link
Collaborator

Look at my comments, then squash and force push

@An-DJ
Copy link
Contributor Author

An-DJ commented Jun 30, 2021

Look at my comments, then squash and force push

Done

return 1;
}

struct prometheus* prometheus;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this to the beginning of the function

struct configuration* config;

config = (struct configuration*) shmem;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create local variables for the size and the memory segment.

Set p_size and p_shmem to 0 and NULL in the beginning of the function, and only assign them if everything is successful

@@ -791,6 +849,11 @@ static void
general_information(int client_fd)
{
char* data = NULL;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

White space

@jesperpedersen
Copy link
Collaborator

Just a few more

@An-DJ
Copy link
Contributor Author

An-DJ commented Jul 1, 2021

Just a few more

Done

@jesperpedersen
Copy link
Collaborator

Thanks for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split the struct Prometheus from struct Configuration
2 participants