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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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.

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.

Race condition when registering ATC tables
2 participants