Skip to content
This repository was archived by the owner on Aug 24, 2022. It is now read-only.

PMM-4648 Rotate slowlog. - #116

Merged
AlekSi merged 20 commits into
masterfrom
PMM-4437-flush-slowlogs
Sep 13, 2019
Merged

AlekSi merged 20 commits into
masterfrom
PMM-4437-flush-slowlogs

Conversation

@askomorokhov

@askomorokhov askomorokhov commented Sep 9, 2019 •

Copy link
Copy Markdown
Contributor

https://jira.percona.com/browse/PMM-4459
https://jira.percona.com/browse/PMM-4437

depends on:
percona/pmm-admin#59
percona/pmm-managed#264
percona/qan-app#322

FB:
Percona-Lab/pmm-submodules#444

usage ex.:
pmm-admin add mysql --username=root --password=secret ps:3306 MySQLSlowLog --size-slow-logs=10MB

Comment thread Gopkg.toml Outdated
name = "github.com/percona/pmm"
branch = "PMM-2.0"
#branch = "PMM-2.0"
branch = "PMM-4437-flush-slowlogs"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Merged with changes

Comment thread agentlocal/mock_client_test.go Outdated
import agentpb "github.com/percona/pmm/api/agentpb"
import mock "github.com/stretchr/testify/mock"
import prometheus "github.com/prometheus/client_golang/prometheus"
import time "time"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please run make init before make gen.

Comment thread agents/mysql/slowlog/slowlog.go Outdated
dsn string
agentID string
slowLogFilePrefix string
sizeSlowLogs uint32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

int64 to drop conversions below.

Comment thread agents/mysql/slowlog/slowlog.go Outdated
return errors.Wrap(err, "cannot rename slow log")
}

_, err = db.ExecContext(ctx, fmt.Sprintf("SET GLOBAL /* %s */ slow_query_log=on", queryTag)) //nolint:gosec

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should do it in defer. If we need to that at all. @rnovikovP

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like modification in the Global setting. we are trying to avoid this

Comment thread agents/mysql/slowlog/slowlog.go Outdated
return errors.Wrap(err, "cannot flush logs")
}

err = os.Rename(slowLogPath, slowLogPath+".old")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should remove it, not rename. ContinuousFileReader will still be able to read removed file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also remove it before FLUSH LOGS. That the whole point of that SQL command.

Comment thread agents/mysql/slowlog/slowlog.go Outdated
} else {
// get the size of slowlog
currSizeSlowLog := fi.Size()
if currSizeSlowLog > int64(s.sizeSlowLogs) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should skip the whole block if sizeSlowLogs <=0


// makeBuckets is a pure function for easier testing.
func makeBuckets(agentID string, res event.Result, periodStart time.Time, periodLengthSecs uint32) []*agentpb.MetricsBucket {
func makeBuckets(agentID string, res event.Result, periodStart time.Time, periodLengthSecs uint32, disableQueryExamples bool) []*agentpb.MetricsBucket {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

cyclomatic complexity 38 of func makeBuckets is high (> 30) (from gocyclo)

@askomorokhov
askomorokhov requested a review from AlekSi September 11, 2019 17:52
@codecov

codecov Bot commented Sep 11, 2019 •

Copy link
Copy Markdown

Codecov Report

Merging #116 into master will decrease coverage by 0.51%.
The diff coverage is 54.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #116      +/-   ##
==========================================
- Coverage    65.9%   65.39%   -0.52%     
==========================================
  Files          53       53              
  Lines        5371     5418      +47     
==========================================
+ Hits         3540     3543       +3     
- Misses       1628     1665      +37     
- Partials      203      210       +7
Flag Coverage Δ
#cover 58.78% <54.05%> (-0.21%) ⬇️
#crosscover 65.39% <54.05%> (-0.52%) ⬇️
Impacted Files Coverage Δ
agents/supervisor/supervisor.go 73.24% <0%> (-0.6%) ⬇️
agents/mysql/perfschema/perfschema.go 57.97% <100%> (+0.41%) ⬆️
agents/mysql/slowlog/slowlog.go 50.78% <58.73%> (-5.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cb0502...3a1a08d. Read the comment docs.

@AlekSi AlekSi assigned AlekSi and unassigned askomorokhov Sep 12, 2019
@AlekSi
AlekSi requested a review from BupycHuk September 12, 2019 19:31
@AlekSi
AlekSi merged commit 0e732d7 into master Sep 13, 2019
@AlekSi
AlekSi deleted the PMM-4437-flush-slowlogs branch September 13, 2019 10:44
isabek pushed a commit to isabek/pmm-agent that referenced this pull request Feb 17, 2021
* PMM-6213 Remove go 1.13 from travis.

* PMM-6213 Add go 1.15.x

* PMM-6213 Add go tip.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants