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

add pending state to ATC tables to avoid duplicate sql attaches #8324

Merged

Conversation

bgirardeau-figma
Copy link
Contributor

@bgirardeau-figma bgirardeau-figma commented May 1, 2024

This reverts #8233 and adds an alternate fix to avoid duplicate ATC table registrations. The idea is to add a pending state to the ATC table that ensures it is skipped in the attachVirtualTables function (called during database initialization) in favor of being added by the SQL plugin's "attach" command.

After the table is registered successfully, it is no longer set to pending, so it is included as expected when the database is reloaded or an ephemeral non-primary instance created.

Fixes: #8323

Manually tested the key scenarios:

  • osquery server that delayed serving config so ATC registered after extension
    • behavior on osquery 5.12.1 was that ATC table threw error when querying
    • behavior with this PR was that ATC table could be queried successfully
  • osquery server that served config quickly so ATC registered before extension
    • behavior on both osquery 5.12.1 and this PR was that ATC table could be queried successfully
    • no duplicate registration error message was reported

@Micah-Kolide
Copy link
Contributor

Micah-Kolide commented May 7, 2024

I pulled this down and tested as well. I like this solution a lot as it's not changing anything around initialization while ensuring each table registration method (atc, extension, internal) handles their relevant tables without overlap like we saw before with attachVirtualTables trying to handle the atc table attach.

directionless
directionless previously approved these changes Jul 3, 2024
Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

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

I've stared at this a bit, and I think it's fine. Might be bugs, but I don't think anything catastrophic.

@directionless directionless merged commit fdd6157 into osquery:master Jul 4, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Race condition when registering ATC tables
4 participants