Skip to content

*:process approximate rows in region#1104

Merged
AndreMouche merged 6 commits into
tikv:masterfrom
AndreMouche:shirly/region_approximate_rows
Jun 12, 2018
Merged

*:process approximate rows in region#1104
AndreMouche merged 6 commits into
tikv:masterfrom
AndreMouche:shirly/region_approximate_rows

Conversation

@AndreMouche
Copy link
Copy Markdown
Member

Hi
This PR process the approximate rows in the region from the heartbeat request.
@nolouch @disksing PTAL

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 8, 2018

CLA assistant check
All committers have signed the CLA.

kind |= (schedule.OpRegion | schedule.OpLeader)
c.Assert(op.Kind()&kind, check.Equals, kind)
}
// Copyright 2017 PingCAP, Inc.
Copy link
Copy Markdown
Contributor

@nolouch nolouch Jun 8, 2018

Choose a reason for hiding this comment

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

why this file has so many changes?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Since I do gofmt with go version go1.10.1 darwin/amd64 ?

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.

It looks like that old file is not well formatted. (apparently make check skips all files in testutil, no idea why...)

Comment thread server/cluster_info.go
saveCache = true
}
if region.ApproximateRows != origin.ApproximateRows {
saveCache = true
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.

Is the ApproximateRows too sensitive to change?

Copy link
Copy Markdown
Member Author

@AndreMouche AndreMouche Jun 12, 2018

Choose a reason for hiding this comment

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

I'm not sure, should we change it @nolouch @disksing ?

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.

I think it should be ok as long as they are only saved to cache (not to etcd).

Comment thread server/schedule/basic_cluster.go Outdated
RegionRows int64
RegionCount int64
LeaderSize int64
LeaderRows int64
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.

I think we can exclude RegionRows and LeaderRows in StoreInfluence.

@disksing
Copy link
Copy Markdown
Contributor

LGTM.

Copy link
Copy Markdown
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

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

LGTM. please update the proto.

Copy link
Copy Markdown
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

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

LGTM

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.

4 participants