Update vendor to the latest and add implementation test for GetPrevRegion #7429

arthurkiller · 2018-08-18T06:53:27Z

What problem does this PR solve?

Update pd and kv proto to the latest according to tikv/pd#1175, which supply the GetPrevRegion function that is really helpful.

What is changed and how it works?

Run dep ensure to update vendor

dep ensure -update github.com/pingcap/pd
dep ensure -update github.com/pingcap/kvproto

Check List

Tests
I have build this fella locally

arthur@ArthursMacBookPro:~/g/s/g/p/tidb:master$ make
CGO_ENABLED=0 go build   -ldflags '-X "github.com/pingcap/tidb/mysql.TiDBReleaseVersion=v2.1.0-beta-252-ga4ab22f2-dirty" -X "github.com/pingcap/tidb/util/printer.TiDBBuildTS=2018-08-18 06:45:09" -X "github.com/pingcap/tidb/util/printer.TiDBGitHash=a4ab22f2730d0274670b585fec7e058636d7b833" -X "github.com/pingcap/tidb/util/printer.TiDBGitBranch=master" -X "github.com/pingcap/tidb/util/printer.GoVersion=go version go1.10.2 darwin/amd64"' -o bin/tidb-server tidb-server/main.go
Build TiDB Server successfully!

Unit test
Integration test
Manual test (add detailed scripts or steps below)

Code changes

func (c *pdClient) GetPrevRegion(ctx context.Context, key []byte) (*metapb.Region, *metapb.Peer, error) {
     region, peer := c.cluster.GetPrevRegionByKey(key)
     return region, peer, nil
}

Side effects

Breaking backward compatibility

Related changes

Need to update the documentation
Need to be included in the release note

sre-bot · 2018-08-18T06:53:32Z

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

siddontang · 2018-08-18T13:30:46Z

Gopkg.lock

+
+[[projects]]
+  branch = "master"
+  name = "github.com/pingcap/tidb"


very strange that tidb contains tidb repo....

fixed in 080cf406

@arthurkiller

The tidb should not be in lock file too.

Maybe you can use dep ensure to remove this?

It is incorrect to remove it in lock file manually.

BTW. I did the same... :(

Maybe auto merge got things wrong

I will clean the work space and do it again

@liukun4515 are you sure do things above will not add tidb into vendor ?

If updating vendor is a troublesome thing for you, we can give a pr to update vendor, and you can implement getPrevregion base on the latest master.
By the way, if TiDB wants to use the getPrevReigon feature, we should add this feature to region_cache.go.

Maybe it's not the time for me to do contribute.XD

Could you pls put up another PR only update things you mentioned?

Ahaa, I get it. I did dep ensure in my local repo without run checkout-pr-branch.sh

so dep add the github.vendor for me.

I will redo again...

liukun4515 · 2018-08-18T14:04:53Z

After updating the vendor, you should run the script ./hack/clean_vendor.sh to clean useless files in vendor.

arthurkiller · 2018-08-20T02:08:18Z

Thx for your reply. I’m just run ‘dep ensure -update github.com/ping cap/pd’ and dep added this dependency automatically. Best regards ✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️ Arthur lee Sent from my iPhone

…

On Aug 20, 2018, at 10:05 AM, siddontang ***@***.***> wrote: @siddontang commented on this pull request. In Gopkg.lock: > [[projects]] branch = "master" name = "github.com/pingcap/pd" packages = ["pd-client"] - revision = "ea6597638eb7f3d7d50248b90e60888352ac8e85" + revision = "ba75bdda52f50df752c42c3f1434d9471e98f75e" + +[[projects]] + branch = "master" + name = "github.com/pingcap/tidb" @arthurkiller The tidb should not be in lock file too. Maybe you can use dep ensure to remove this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

arthurkiller · 2018-08-20T02:09:39Z

Maybe remove it manually is a possible solution? Best regards ✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️✉️ Arthur lee Sent from my iPhone

…

On Aug 20, 2018, at 10:05 AM, siddontang ***@***.***> wrote: @siddontang commented on this pull request. In Gopkg.lock: > [[projects]] branch = "master" name = "github.com/pingcap/pd" packages = ["pd-client"] - revision = "ea6597638eb7f3d7d50248b90e60888352ac8e85" + revision = "ba75bdda52f50df752c42c3f1434d9471e98f75e" + +[[projects]] + branch = "master" + name = "github.com/pingcap/tidb" @arthurkiller The tidb should not be in lock file too. Maybe you can use dep ensure to remove this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

arthurkiller · 2018-08-20T02:36:41Z

Ok, I will checkout the vendor .toml and .lock` and run ensure again.

arthurkiller · 2018-08-20T03:11:06Z

Done, this time things going well

siddontang · 2018-08-21T02:26:46Z

Gopkg.lock

  revision = "51ce9a71510a58bad5ae66ddd278ef28762a1550"

 [[projects]]
  name = "github.com/go-sql-driver/mysql"
  packages = ["."]
  revision = "3955978caca48c1658a4bb7a9c6a0f084e326af3"

-[[projects]]
-  name = "github.com/gogo/protobuf"


do we not use gogo now? @tiancaiamao

I think update pd and kvproto should not affect those packages.

I didn't know what's wrong but each time I run

dep ensure -update github.com/pingcap/pd

the gogo package will be replaced with google/protobuffer

@liukun4515 FYI

siddontang · 2018-08-21T02:26:56Z

Gopkg.lock

@@ -172,15 +162,6 @@
  packages = ["."]
  revision = "7a24ed77b2efb460c1468b7dc917821c66e80e55"

-[[projects]]
-  name = "github.com/opentracing/basictracer-go"


same here @tiancaiamao

It's still in use.

@liukun4515 FYI

siddontang · 2018-08-21T02:33:48Z

CI failed @arthurkiller

sre-bot · 2018-08-21T14:33:31Z

Hi contributor, thanks for your PR.

This patch needs to be approved by someone of admins. They should reply with "/ok-to-test" to accept this PR for running test automatically.

arthurkiller · 2018-08-21T14:36:00Z

Warning: the following project(s) have [[constraint]] stanzas in Gopkg.toml:

  ✗  github.com/opentracing/basictracer-go

However, these projects are not direct dependencies of the current project:
they are not imported in any .go files, nor are they in the 'required' list in
Gopkg.toml. Dep only applies [[constraint]] rules to direct dependencies, so
these rules will have no effect.

ridiculous

arthurkiller · 2018-08-21T14:45:08Z

@tiancaiamao @liukun4515 I think the best way is run the dep update by you guys.

I did not know why I just failed. :(

siddontang · 2018-08-21T16:50:38Z

PTAL @liukun4515

liukun4515 · 2018-08-22T02:16:53Z

Do I need to continue this job by updating the vendor? @arthurkiller @siddontang

arthurkiller · 2018-08-22T02:41:08Z

@liukun4515 Thats's very nice of you

arthurkiller · 2018-08-22T03:45:49Z

@siddontang @tiancaiamao Hey dude. @liukun4515 has do update on his own but unfortunately we get the same results.

R U SURE these dependency is still in use?

siddontang · 2018-08-22T18:04:16Z

@liukun4515

You can file another PR

liukun4515 · 2018-08-23T01:00:33Z

The version of tipb is incompatible between tidb and kvproto.
When we update the kvproto in tidb, the version of tipb will be updated depending on the kvproto.
Related pr is tipb and tidb.
This may be the reason for the CI failure.
@arthurkiller
@siddontang
I will solve this problem first.
Thanks @tiancaiamao

arthurkiller · 2019-08-26T08:34:33Z

closed via #10152

siddontang reviewed Aug 18, 2018

View reviewed changes

siddontang reviewed Aug 21, 2018

View reviewed changes

arthurkiller closed this Aug 21, 2018

arthurkiller mentioned this pull request Mar 5, 2019

Implement reverse iterator #9564

Closed

sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update vendor to the latest and add implementation test for GetPrevRegion #7429

Update vendor to the latest and add implementation test for GetPrevRegion #7429

arthurkiller commented Aug 18, 2018 •

edited

Loading

sre-bot commented Aug 18, 2018

siddontang Aug 18, 2018

liukun4515 Aug 18, 2018

arthurkiller Aug 19, 2018

siddontang Aug 20, 2018

liukun4515 Aug 20, 2018

arthurkiller Aug 20, 2018 •

edited

Loading

arthurkiller Aug 20, 2018

liukun4515 Aug 20, 2018

arthurkiller Aug 20, 2018 •

edited

Loading

arthurkiller Aug 20, 2018 •

edited

Loading

liukun4515 commented Aug 18, 2018

arthurkiller commented Aug 20, 2018 via email

arthurkiller commented Aug 20, 2018 via email

arthurkiller commented Aug 20, 2018

arthurkiller commented Aug 20, 2018

siddontang Aug 21, 2018

tiancaiamao Aug 21, 2018

arthurkiller Aug 21, 2018 •

edited

Loading

arthurkiller Aug 22, 2018

siddontang Aug 21, 2018

tiancaiamao Aug 21, 2018

arthurkiller Aug 22, 2018

siddontang commented Aug 21, 2018

sre-bot commented Aug 21, 2018

arthurkiller commented Aug 21, 2018 •

edited

Loading

arthurkiller commented Aug 21, 2018

siddontang commented Aug 21, 2018

liukun4515 commented Aug 22, 2018 •

edited

Loading

arthurkiller commented Aug 22, 2018

arthurkiller commented Aug 22, 2018

siddontang commented Aug 22, 2018

liukun4515 commented Aug 23, 2018 •

edited

Loading

arthurkiller commented Aug 26, 2019

Update vendor to the latest and add implementation test for GetPrevRegion #7429

Update vendor to the latest and add implementation test for GetPrevRegion #7429

Conversation

arthurkiller commented Aug 18, 2018 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

sre-bot commented Aug 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthurkiller Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthurkiller Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

arthurkiller Aug 20, 2018 • edited Loading

Choose a reason for hiding this comment

liukun4515 commented Aug 18, 2018

arthurkiller commented Aug 20, 2018 via email

arthurkiller commented Aug 20, 2018 via email

arthurkiller commented Aug 20, 2018

arthurkiller commented Aug 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arthurkiller Aug 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siddontang commented Aug 21, 2018

sre-bot commented Aug 21, 2018

arthurkiller commented Aug 21, 2018 • edited Loading

arthurkiller commented Aug 21, 2018

siddontang commented Aug 21, 2018

liukun4515 commented Aug 22, 2018 • edited Loading

arthurkiller commented Aug 22, 2018

arthurkiller commented Aug 22, 2018

siddontang commented Aug 22, 2018

liukun4515 commented Aug 23, 2018 • edited Loading

arthurkiller commented Aug 26, 2019

arthurkiller commented Aug 18, 2018 •

edited

Loading

arthurkiller Aug 20, 2018 •

edited

Loading

arthurkiller Aug 20, 2018 •

edited

Loading

arthurkiller Aug 20, 2018 •

edited

Loading

arthurkiller Aug 21, 2018 •

edited

Loading

arthurkiller commented Aug 21, 2018 •

edited

Loading

liukun4515 commented Aug 22, 2018 •

edited

Loading

liukun4515 commented Aug 23, 2018 •

edited

Loading