Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

[elastic] Move the deps downloading into 'intialize' request handle #79

Merged

Conversation

movie-travel-code
Copy link

The original implementation relies on 'go list' to download the deps.
'go list' is called by other request handler indirectly. This approach
is unpredictable and 'go list' is black box to us. So it's better
download the deps manually in 'ManageDeps', i.e. inside 'initialize'
request handler.

@AppVeyorBot
Copy link

@@ -215,6 +214,13 @@ func (s ElasticServer) ManageDeps(folders *[]protocol.WorkspaceFolder) error {
}
}
}
for _, folder := range *folders {
Copy link

Choose a reason for hiding this comment

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

should this guarded by the downloading deps flag?

Copy link
Author

Choose a reason for hiding this comment

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

Yea, will do

@movie-travel-code movie-travel-code force-pushed the initialize_download_deps branch from b948c72 to 64bd846 Compare August 21, 2019 15:30
@AppVeyorBot
Copy link

@movie-travel-code
Copy link
Author

movie-travel-code commented Aug 21, 2019

Move the deps download into 'initialize' handler. And in order to disable the deps download by go list ... automatically, put the GO111MODULE=off to every view, i.e. workspace folder. So for now, there is a strong guarantee that only enables installGoDependency=true can enable the deps download.

Result of index github.com/etcd-io/etcd.

| | symbol count | time |
|:--|:--|:--|
| GO111MODULE=off | 12617 | 148.828s |
| installGoDependency=true | 15074 | 87.092s |

GO111MODULE=off is not the perfect approach to control deps download. There are symbols missing and don't support different packages jump in the same repo.

The official option to disable network access is GOPROXY=off, related issue golang/go#32337.

@movie-travel-code movie-travel-code changed the title [elastic] Move the deps downloading into 'intialize' request handle [elastic] [WIP] Move the deps downloading into 'intialize' request handle Aug 21, 2019
@movie-travel-code movie-travel-code force-pushed the initialize_download_deps branch from 64bd846 to 0a0f0d7 Compare August 22, 2019 14:24
@AppVeyorBot
Copy link

@movie-travel-code
Copy link
Author

movie-travel-code commented Aug 22, 2019

summary

Under module mode, any standard go command, include go list ..., will automatically download the dependencies, see https://github.com/golang/go/wiki/Modules#daily-workflow. Since we convert the repos into modules, when langserver try to call go list ... it will download the deps automatically.

Code has an option installGoDependency to enable the deps download.

  • If installGoDependency set to false or not set, we will append GOPROXY=off and ENABLEVENDOR=on to the os environment, GOPROXY=off is a strong guarantee that disables the network access. ENABLEVENDOR=on is a flag created manually to tell go list ... use mod=vendor to find deps from vendor folder.
  • If installGoDependency set to true, langserver will download the deps to data/code/gopath/pkg/mod in the initialize request handler. And go list ... will find the deps from data/code/gopath/pkg/mod.

Test results

repos symbols total time time per go file
mongo-go-driver module mode
contain vendor folder, 949-583 .go
5981 53.213s 145.4ms
mongo-go-driver vendor mode
_contain vendor folder, 949 .go
5460 33.662s 92.0ms
etcd module mode
contain vendor folder, 1888-962 .go
15074 66.766s 72.1ms
etcd vendor mode
contain vendor folder, 1888 .go
15074 84.813s 91.6ms
Gin module mode
just vendor.json, 73 .go
1339 9.639s 132ms
Gin vendor mode
just vendor.json, 73 .go
756 5.592s 76.6ms
logrus module mode
no vendor folder, 38 .go
477 7.96s 209.5ms
logrus vendor mode
no vendor folder, 38 .go
320 6.8s 179ms
tidb module mode
no vendor, 824 .go
23795 92.896s 112.7ms
tidb vendor mode
no vendor, 824 .go
18728 58.333ms 71.4ms
docker-engine module mode
contain vendor folder, 4921-3272 .go
13158 224.459s 136.1ms
docker-engine vendor mode
contain vendor folder, 4921-3272 .go
11637 158.486s 96.1ms

Test result of Kubernetes:

Symbol Time Time per go file go list ... time
12685-5016 .go file 116707 1005.466s 131.107ms 637.859s( 63.44% )

The test for https://github.com/prometheus/prometheus got stuck in the end.

@movie-travel-code movie-travel-code force-pushed the initialize_download_deps branch from 0a0f0d7 to 59b5269 Compare August 23, 2019 05:48
@AppVeyorBot
Copy link

@movie-travel-code movie-travel-code force-pushed the initialize_download_deps branch from 59b5269 to 62da059 Compare August 23, 2019 14:59
@AppVeyorBot
Copy link

@movie-travel-code movie-travel-code force-pushed the initialize_download_deps branch from 62da059 to 6b319e4 Compare August 24, 2019 12:10
@AppVeyorBot
Copy link

@movie-travel-code movie-travel-code changed the title [elastic] [WIP] Move the deps downloading into 'intialize' request handle [elastic] Move the deps downloading into 'intialize' request handle Aug 24, 2019
@movie-travel-code
Copy link
Author

@zfy0701 this PR has come to the final stage, for now, the option installGoDependency is the only approach to control the deps download.

Related PR: elastic/kibana#43940

The original implementation relies on 'go list' to download the deps.
And 'go list' will be called by the request handler indirectly.
This approach is unpredictable and 'go list' is black box to us. So it's
better download the deps manually in 'ManageDeps', i.e. inside
'initialize' request handler.

If the client sends the initialize option `installGoDependency: true`,
download the deps in `initialize` handler.

If the client doesn't send the initialize option `installGoDependency: true`,
disable the network access by `GOPROXY=off` and find the deps from vendor
folder.
@movie-travel-code movie-travel-code force-pushed the initialize_download_deps branch from 6b319e4 to 097700a Compare August 26, 2019 09:15
@AppVeyorBot
Copy link

@movie-travel-code movie-travel-code merged commit c6a3f8a into elastic:master Aug 26, 2019
movie-travel-code pushed a commit to movie-travel-code/tools that referenced this pull request Aug 28, 2019
Note: The index time increase almost 15% than
elastic#79
movie-travel-code pushed a commit that referenced this pull request Aug 28, 2019
Note: The index time increase almost 15% than
#79
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants