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

Fix negative table size calculation when socket tracer disabled via stirling flag #1894

Merged
merged 5 commits into from May 20, 2024

Conversation

benkilimnik
Copy link
Member

@benkilimnik benkilimnik commented May 6, 2024

Summary: When some or all of the tables http_events, stirling_error, probe_status, proc_exit_events are disabled via stirling flags (PL_STIRLING_SOURCES=...) the other_table_size calculation returns a negative output if num_tables < 4. It also attempts division by zero when num_tables = 4

  int64_t other_table_size = (memory_limit - http_table_size - stirling_error_table_size -
                              probe_status_table_size - proc_exit_events_table_size) /
                             (num_tables - 4);

This PR streamlines the table size calculation and fixes the edge condition.

Type of change: /kind bug

Test Plan: deployed to node and manually checked calculated table sizes.

@benkilimnik benkilimnik changed the title Fix negative table size calculation when socket tracer disabled via stirling flag Fix negative table size calculation when socket tracer disabled via stirling flag for standalone pem May 6, 2024
@benkilimnik benkilimnik requested review from a team May 6, 2024 22:16
@benkilimnik benkilimnik marked this pull request as ready for review May 6, 2024 22:16
@benkilimnik benkilimnik requested a review from a team as a code owner May 6, 2024 22:16
Signed-off-by: Benjamin Kilimnik <bkilimnik@pixielabs.ai>
@benkilimnik benkilimnik force-pushed the standalone-pem-tablesize-calc branch from 548b6f2 to c2c41e4 Compare May 9, 2024 02:10
…en remaining_memory < 0

Signed-off-by: Benjamin Kilimnik <bkilimnik@pixielabs.ai>
@benkilimnik
Copy link
Member Author

Just pushed an update to also fix table size calculations in pem_manager.cc and add an error condition for when the pem flags are set such that there is not enough space for the default tables.

  if (remaining_memory < 0) {
    return error::Internal("Table store data limit is too low to store the tables.");
  }

Signed-off-by: Benjamin Kilimnik <bkilimnik@pixielabs.ai>
Signed-off-by: Benjamin Kilimnik <bkilimnik@pixielabs.ai>
Signed-off-by: Benjamin Kilimnik <bkilimnik@pixielabs.ai>
@benkilimnik benkilimnik force-pushed the standalone-pem-tablesize-calc branch from 873c312 to bbf38c1 Compare May 10, 2024 03:02
@benkilimnik benkilimnik changed the title Fix negative table size calculation when socket tracer disabled via stirling flag for standalone pem Fix negative table size calculation when socket tracer disabled via stirling flag May 15, 2024
@ddelnano ddelnano merged commit d8aab71 into pixie-io:main May 20, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants