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

PT-2133 - added basic support for MongoDB to pt-stalk #163

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

fipar
Copy link
Contributor

@fipar fipar commented Feb 11, 2017

This branch adds basic MongoDB support to pt-stalk, adding the --mongo option, which:

  • Disables the MySQL collectors,
  • Collects db.currentOp(), db.isMaster(), sh.status(), rs.status() and db.serverStatus()
  • One of sh.status or rs.status will fail as they are mutually exclusive, but the collect_mongo function does not check this and runs both anyway
  • Provides trg_mongo_default as the default mongo trigger (count of 'inprog' current operations) if --function is not specified

It includes no tests of these functions as it does not provide a mongodb sandbox, but it does not break any of the existing pt-stalk tests.

@dbmurphy
Copy link

This should be moved to the new PT in go tools

@svetasmirnova svetasmirnova changed the base branch from 3.0-rc to 3.x December 2, 2022 10:40
@svetasmirnova svetasmirnova mentioned this pull request Dec 2, 2022
@svetasmirnova svetasmirnova changed the title added basic support for MongoDB to pt-stalk PT-2133 - added basic support for MongoDB to pt-stalk Dec 2, 2022
@fipar fipar requested a review from svetasmirnova as a code owner December 22, 2023 20:18
Copy link
Collaborator

@svetasmirnova svetasmirnova left a comment

Choose a reason for hiding this comment

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

I am sorry it took too long to review this but I have a few suggestions.

  1. I believe that instead of having MySQL and MongoDB collections mutually exclusive, it is better to not mix up MySQL and MongoDB connection options, rather collect their data independently.
  2. Since we have option --system-only now, maybe it makes sense to implement more complicated logic and let users choose if they want to collect: system, mysql and mongo info, only system, only system and mysql, only system and mongo, only mysql, only mongo, and (why not?) only mysql and mongo
  3. Please add a test case. You will find examples in t/pt-stalk, t/lib/bash/collect.t, and t/lib/bash/collect.sh

@dbmurphy
Copy link

That seems overly complex maybe system and db-engine should be independent.

--system --db-engine mongo where system is a flag and db-engine is a list with specific connection variables prefixed for each engine (keeping no prefix assuming MySQL and system-only mapped to --system --db-engine none

Really we should be talking about a parent command which calls one of N child commands based on db-engine. But that would be a much bigger redesign but more long term scalable

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.

3 participants