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

Implement server stats and self-monitoring #1936

Merged
merged 26 commits into from
Mar 16, 2015
Merged

Implement server stats and self-monitoring #1936

merged 26 commits into from
Mar 16, 2015

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Mar 12, 2015

No description provided.

@otoolep
Copy link
Contributor Author

otoolep commented Mar 12, 2015

$ curl -G http://localhost:8086/query?pretty=true --data-urlencode "q=SHOW STATS"
{
    "results": [
        {
            "series": [
                {
                    "name": "server",
                    "columns": [
                        "batchWriteRx",
                        "broadcastMessageRx",
                        "broadcastMessageTx",
                        "pointWriteRx",
                        "queryRx",
                        "queryStatementRx",
                        "rowsReturned",
                        "shardWriteByte",
                        "shardWriteTime",
                        "shardsCreated",
                        "writeSeriesMessageRx",
                        "writeSeriesMessageTx"
                    ],
                    "values": [
                        [
                            15
                        ],
                        [
                            74
                        ],
                        [
                            64
                        ],
                        [
                            47
                        ],
                        [
                            78
                        ],
                        [
                            78
                        ],
                        [
                            40
                        ],
                        [
                            1350
                        ],
                        [
                            468990206
                        ],
                        [
                            9
                        ],
                        [
                            15
                        ],
                        [
                            15
                        ]
                    ]
                }
            ]
        }
    ]
}

@otoolep otoolep force-pushed the server_stats2 branch 3 times, most recently from 588c8e7 to 3fbc155 Compare March 13, 2015 23:59
@beckettsean beckettsean added this to the 0.9.0 milestone Mar 14, 2015
log.Fatalf("failed to create retention policy for internal statistics: %s", err.Error())
}

s.StartSelfMonitoring(database, policy, interval)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing an interval := config.Statistics.WriteInterval?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good -- I just fixed that on my local copy and see you spotted it. Fix coming.

@beckettsean beckettsean modified the milestones: 0.9.0, Next Release Mar 14, 2015
@otoolep
Copy link
Contributor Author

otoolep commented Mar 14, 2015

There is 1 piece remaining for this, the ability to query a different server. This is being tracked by #1945

@toddboom toddboom modified the milestones: Next Point Release, 0.9.0 Mar 14, 2015
@otoolep
Copy link
Contributor Author

otoolep commented Mar 14, 2015

Green build.


go func() {
for {
time.Sleep(interval)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the self-monitoring is a form of instrumentation, we should probably be using a time.Ticker.

For a good demonstration of the difference between the two, compare the output of using time.Sleep vs. using time.NewTicker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes, I know about the tickers. I agree it would be nice to be precise with this stuff.

@otoolep
Copy link
Contributor Author

otoolep commented Mar 14, 2015

Updated to use time.Ticker.

@pauldix pauldix mentioned this pull request Mar 14, 2015
@toddboom
Copy link
Contributor

@otoolep Would you mind rebasing this? If you think it's ready to go, I'll merge it in for RC12.

@otoolep
Copy link
Contributor Author

otoolep commented Mar 15, 2015

I have rebased this and the build is green. I have opened a ticket for the unrelated Travis failure. #1962

I consider this code ready to merge, though there is still significant more work to do in terms of the stats we need to collect. We also "lost" some stats due to the Raft refactor, since some stats are now only available at a lower-level than the server object. I will need to take a look at how to gather those important stats within the new framework.

This code has not been reviewed, so please review before merging.

@toddboom
Copy link
Contributor

Awesome, this looks good to me. I'll give everyone else a chance to review and then I'll merge in a couple hours.

toddboom added a commit that referenced this pull request Mar 16, 2015
Implement server stats and self-monitoring
@toddboom toddboom merged commit 86ede78 into master Mar 16, 2015
@toddboom toddboom deleted the server_stats2 branch March 16, 2015 03:48
mark-rushakoff pushed a commit that referenced this pull request Jan 11, 2019
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

4 participants