From 5f26cbc31755c7b09b07a9c196352cef03d7e657 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Tue, 19 May 2020 20:14:25 -0500 Subject: [PATCH 01/11] Consolidate janitors. --- cmd/precise-code-intel-api-server/env.go | 12 -- .../internal/janitor/janitor.go | 68 ---------- .../internal/janitor/janitor_test.go | 17 --- .../internal/janitor/metrics.go | 27 ---- ...cessed_uploads_without_bundle_file_test.go | 119 ------------------ .../internal/server/handler.go | 45 ------- cmd/precise-code-intel-api-server/main.go | 5 - .../internal/janitor/free_space.go | 33 ++--- .../internal/janitor/janitor.go | 15 ++- .../internal/janitor/metrics.go | 8 ++ .../janitor/remove_orphaned_bundle_files.go | 18 +-- ...e_processed_uploads_without_bundle_file.go | 34 +---- ...cessed_uploads_without_bundle_file_test.go | 52 ++++++++ .../internal/paths/exists.go | 15 +++ .../internal/server/handler.go | 22 +--- .../internal/server/util.go | 13 -- cmd/precise-code-intel-bundle-manager/main.go | 23 +++- dev/check/go-dbconn-import.sh | 2 +- .../codeintel/lsifserver/client/internal.go | 61 --------- monitoring/precise_code_intel_api_server.go | 26 ---- .../precise_code_intel_bundle_manager.go | 12 ++ 21 files changed, 142 insertions(+), 485 deletions(-) delete mode 100644 cmd/precise-code-intel-api-server/internal/janitor/janitor.go delete mode 100644 cmd/precise-code-intel-api-server/internal/janitor/janitor_test.go delete mode 100644 cmd/precise-code-intel-api-server/internal/janitor/metrics.go delete mode 100644 cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file_test.go rename cmd/{precise-code-intel-api-server => precise-code-intel-bundle-manager}/internal/janitor/remove_processed_uploads_without_bundle_file.go (57%) create mode 100644 cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file_test.go create mode 100644 cmd/precise-code-intel-bundle-manager/internal/paths/exists.go delete mode 100644 internal/codeintel/lsifserver/client/internal.go diff --git a/cmd/precise-code-intel-api-server/env.go b/cmd/precise-code-intel-api-server/env.go index 29771e07712e0b..a29e7ddb0e25d6 100644 --- a/cmd/precise-code-intel-api-server/env.go +++ b/cmd/precise-code-intel-api-server/env.go @@ -2,14 +2,12 @@ package main import ( "log" - "time" "github.com/sourcegraph/sourcegraph/internal/env" ) var ( rawBundleManagerURL = env.Get("PRECISE_CODE_INTEL_BUNDLE_MANAGER_URL", "", "HTTP address for internal LSIF bundle manager server.") - rawJanitorInterval = env.Get("PRECISE_CODE_INTEL_JANITOR_INTERVAL", "30m", "Interval between cleanup runs.") ) // mustGet returns the non-empty version of the given raw value fatally logs on failure. @@ -20,13 +18,3 @@ func mustGet(rawValue, name string) string { return rawValue } - -// mustParseInterval returns the interval version of the given raw value fatally logs on failure. -func mustParseInterval(rawValue, name string) time.Duration { - d, err := time.ParseDuration(rawValue) - if err != nil { - log.Fatalf("invalid duration %q for %s: %s", rawValue, name, err) - } - - return d -} diff --git a/cmd/precise-code-intel-api-server/internal/janitor/janitor.go b/cmd/precise-code-intel-api-server/internal/janitor/janitor.go deleted file mode 100644 index ffa51c54729cf2..00000000000000 --- a/cmd/precise-code-intel-api-server/internal/janitor/janitor.go +++ /dev/null @@ -1,68 +0,0 @@ -package janitor - -import ( - "sync" - "time" - - "github.com/inconshreveable/log15" - "github.com/pkg/errors" - "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" - "github.com/sourcegraph/sourcegraph/internal/codeintel/db" -) - -type Janitor struct { - db db.DB - bundleManagerClient client.BundleManagerClient - janitorInterval time.Duration - metrics JanitorMetrics - done chan struct{} - once sync.Once -} - -func New( - db db.DB, - bundleManagerClient client.BundleManagerClient, - janitorInterval time.Duration, - metrics JanitorMetrics, -) *Janitor { - return &Janitor{ - db: db, - bundleManagerClient: bundleManagerClient, - janitorInterval: janitorInterval, - metrics: metrics, - done: make(chan struct{}), - } -} - -// Run periodically performs a best-effort cleanup process. See the following methods -// for more specifics: removeProcessedUploadsWithoutBundleFile. -func (j *Janitor) Run() { - for { - if err := j.run(); err != nil { - j.metrics.Errors.Inc() - log15.Error("Failed to run janitor process", "err", err) - } - - select { - case <-time.After(j.janitorInterval): - case <-j.done: - return - } - } -} - -func (j *Janitor) Stop() { - j.once.Do(func() { - close(j.done) - }) -} - -func (j *Janitor) run() error { - // TODO(efritz) - use cancellable context for API calls - - if err := j.removeProcessedUploadsWithoutBundleFile(); err != nil { - return errors.Wrap(err, "janitor.removeProcessedUploadsWithoutBundle") - } - - return nil -} diff --git a/cmd/precise-code-intel-api-server/internal/janitor/janitor_test.go b/cmd/precise-code-intel-api-server/internal/janitor/janitor_test.go deleted file mode 100644 index 1ed7bd6e94563e..00000000000000 --- a/cmd/precise-code-intel-api-server/internal/janitor/janitor_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package janitor - -import ( - "flag" - "os" - "testing" - - "github.com/inconshreveable/log15" -) - -func TestMain(m *testing.M) { - flag.Parse() - if !testing.Verbose() { - log15.Root().SetHandler(log15.DiscardHandler()) - } - os.Exit(m.Run()) -} diff --git a/cmd/precise-code-intel-api-server/internal/janitor/metrics.go b/cmd/precise-code-intel-api-server/internal/janitor/metrics.go deleted file mode 100644 index 8d2ed1e3250f75..00000000000000 --- a/cmd/precise-code-intel-api-server/internal/janitor/metrics.go +++ /dev/null @@ -1,27 +0,0 @@ -package janitor - -import "github.com/prometheus/client_golang/prometheus" - -type JanitorMetrics struct { - UploadRecordsRemoved prometheus.Counter - Errors prometheus.Counter -} - -func NewJanitorMetrics(r prometheus.Registerer) JanitorMetrics { - uploadRecordsRemoved := prometheus.NewCounter(prometheus.CounterOpts{ - Name: "src_api_server_janitor_upload_records_removed_total", - Help: "Total number of processed upload records removed (with no corresponding bundle file)", - }) - r.MustRegister(uploadRecordsRemoved) - - errors := prometheus.NewCounter(prometheus.CounterOpts{ - Name: "src_api_server_janitor_errors_total", - Help: "Total number of errors when running the janitor", - }) - r.MustRegister(errors) - - return JanitorMetrics{ - UploadRecordsRemoved: uploadRecordsRemoved, - Errors: errors, - } -} diff --git a/cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file_test.go b/cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file_test.go deleted file mode 100644 index 48fb37b7fafb6c..00000000000000 --- a/cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file_test.go +++ /dev/null @@ -1,119 +0,0 @@ -package janitor - -import ( - "context" - "sort" - "testing" - - "github.com/google/go-cmp/cmp" - bundlemocks "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/mocks" - dbmocks "github.com/sourcegraph/sourcegraph/internal/codeintel/db/mocks" - "github.com/sourcegraph/sourcegraph/internal/metrics" -) - -func TestRemoveProcessedUploadsWithoutBundleFile(t *testing.T) { - mockDB := dbmocks.NewMockDB() - mockBundleManagerClient := bundlemocks.NewMockBundleManagerClient() - mockDB.GetDumpIDsFunc.SetDefaultReturn([]int{1, 2, 3, 4, 5}, nil) - mockBundleManagerClient.ExistsFunc.SetDefaultReturn(map[int]bool{ - 1: true, - 2: false, - 3: true, - 4: false, - 5: true, - }, nil) - - j := &Janitor{ - db: mockDB, - bundleManagerClient: mockBundleManagerClient, - metrics: NewJanitorMetrics(metrics.TestRegisterer), - } - - if err := j.removeProcessedUploadsWithoutBundleFile(); err != nil { - t.Fatalf("unexpected error removing processed uploads without bundle files: %s", err) - } - - if len(mockBundleManagerClient.ExistsFunc.History()) != 1 { - t.Errorf("unexpected number of Exists calls. want=%d have=%d", 1, len(mockBundleManagerClient.ExistsFunc.History())) - } else { - call := mockBundleManagerClient.ExistsFunc.History()[0] - - if diff := cmp.Diff([]int{1, 2, 3, 4, 5}, call.Arg1); diff != "" { - t.Errorf("unexpected dump ids (-want +got):\n%s", diff) - } - } - - if len(mockDB.DeleteUploadByIDFunc.History()) != 2 { - t.Errorf("unexpected number of DeleteUploadByID calls. want=%d have=%d", 2, len(mockDB.DeleteUploadByIDFunc.History())) - } else { - ids := []int{ - mockDB.DeleteUploadByIDFunc.History()[0].Arg1, - mockDB.DeleteUploadByIDFunc.History()[1].Arg1, - } - sort.Ints(ids) - - if diff := cmp.Diff([]int{2, 4}, ids); diff != "" { - t.Errorf("unexpected dump ids (-want +got):\n%s", diff) - } - } -} - -func TestRemoveProcessedUploadsWithoutBundleMaxRequestBatchSize(t *testing.T) { - var ids []int - for i := 1; i < 255; i++ { - ids = append(ids, i) - } - - mockDB := dbmocks.NewMockDB() - mockBundleManagerClient := bundlemocks.NewMockBundleManagerClient() - mockDB.GetDumpIDsFunc.SetDefaultReturn(ids, nil) - mockBundleManagerClient.ExistsFunc.SetDefaultHook(func(ctx context.Context, ids []int) (map[int]bool, error) { - existsMap := map[int]bool{} - for _, id := range ids { - existsMap[id] = id%2 == 0 - } - return existsMap, nil - }) - - j := &Janitor{ - db: mockDB, - bundleManagerClient: mockBundleManagerClient, - metrics: NewJanitorMetrics(metrics.TestRegisterer), - } - - if err := j.removeProcessedUploadsWithoutBundleFile(); err != nil { - t.Fatalf("unexpected error removing processed uploads without bundle files: %s", err) - } - - var idArgs [][]int - for _, call := range mockBundleManagerClient.ExistsFunc.History() { - idArgs = append(idArgs, call.Arg1) - } - - var allArgs []int - for _, args := range idArgs { - if len(args) > BundleBatchSize { - t.Errorf("unexpected large slice: want < %d have=%d", BundleBatchSize, len(args)) - } - - allArgs = append(allArgs, args...) - } - sort.Ints(allArgs) - - if diff := cmp.Diff(ids, allArgs); diff != "" { - t.Errorf("unexpected flattened arguments to statesFn (-want +got):\n%s", diff) - } - - if len(mockDB.DeleteUploadByIDFunc.History()) != 127 { - t.Errorf("unexpected number of DeleteUploadByID calls. want=%d have=%d", 128, len(mockDB.DeleteUploadByIDFunc.History())) - } -} - -func TestBatchIntSlice(t *testing.T) { - batches := batchIntSlice([]int{1, 2, 3, 4, 5, 6, 7, 8, 9}, 2) - expected := [][]int{{1, 2}, {3, 4}, {5, 6}, {7, 8}, {9}} - - if diff := cmp.Diff(expected, batches); diff != "" { - t.Errorf("unexpected batch layout (-want +got):\n%s", diff) - } -} diff --git a/cmd/precise-code-intel-api-server/internal/server/handler.go b/cmd/precise-code-intel-api-server/internal/server/handler.go index 00153ed2093817..ee57766b34b0a0 100644 --- a/cmd/precise-code-intel-api-server/internal/server/handler.go +++ b/cmd/precise-code-intel-api-server/internal/server/handler.go @@ -1,7 +1,6 @@ package server import ( - "encoding/json" "fmt" "net/http" @@ -25,8 +24,6 @@ func (s *Server) handler() http.Handler { mux.Path("/definitions").Methods("GET").HandlerFunc(s.handleDefinitions) mux.Path("/references").Methods("GET").HandlerFunc(s.handleReferences) mux.Path("/hover").Methods("GET").HandlerFunc(s.handleHover) - mux.Path("/uploads").Methods("POST").HandlerFunc(s.handleUploads) - mux.Path("/prune").Methods("POST").HandlerFunc(s.handlePrune) mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) @@ -234,45 +231,3 @@ func (s *Server) handleHover(w http.ResponseWriter, r *http.Request) { writeJSON(w, map[string]interface{}{"text": text, "range": rn}) } } - -// POST /uploads -func (s *Server) handleUploads(w http.ResponseWriter, r *http.Request) { - payload := struct { - IDs []int `json:"ids"` - }{} - if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { - log15.Error("Failed to read request body", "error", err) - http.Error(w, fmt.Sprintf("failed to read request body: %s", err.Error()), http.StatusInternalServerError) - return - } - - states, err := s.db.GetStates(r.Context(), payload.IDs) - if err != nil { - log15.Error("Failed to retrieve upload states", "error", err) - http.Error(w, fmt.Sprintf("failed to retrieve upload states: %s", err.Error()), http.StatusInternalServerError) - return - } - - pairs := []interface{}{} - for k, v := range states { - pairs = append(pairs, []interface{}{k, v}) - } - - writeJSON(w, map[string]interface{}{"type": "map", "value": pairs}) -} - -// POST /prune -func (s *Server) handlePrune(w http.ResponseWriter, r *http.Request) { - id, prunable, err := s.db.DeleteOldestDump(r.Context()) - if err != nil { - log15.Error("Failed to prune upload", "error", err) - http.Error(w, fmt.Sprintf("failed to prune upload: %s", err.Error()), http.StatusInternalServerError) - return - } - - if prunable { - writeJSON(w, map[string]interface{}{"id": id}) - } else { - writeJSON(w, map[string]interface{}{"id": nil}) - } -} diff --git a/cmd/precise-code-intel-api-server/main.go b/cmd/precise-code-intel-api-server/main.go index a660e137e40eb3..3d003907403565 100644 --- a/cmd/precise-code-intel-api-server/main.go +++ b/cmd/precise-code-intel-api-server/main.go @@ -10,7 +10,6 @@ import ( "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/api" - "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/janitor" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/server" bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" "github.com/sourcegraph/sourcegraph/internal/codeintel/db" @@ -30,7 +29,6 @@ func main() { var ( bundleManagerURL = mustGet(rawBundleManagerURL, "PRECISE_CODE_INTEL_BUNDLE_MANAGER_URL") - janitorInterval = mustParseInterval(rawJanitorInterval, "PRECISE_CODE_INTEL_JANITOR_INTERVAL") ) observationContext := &observation.Context{ @@ -43,11 +41,8 @@ func main() { bundleManagerClient := bundles.New(bundleManagerURL) codeIntelAPI := api.NewObserved(api.New(db, bundleManagerClient, gitserver.DefaultClient), observationContext) server := server.New(db, bundleManagerClient, codeIntelAPI) - janitorMetrics := janitor.NewJanitorMetrics(prometheus.DefaultRegisterer) - janitor := janitor.New(db, bundleManagerClient, janitorInterval, janitorMetrics) go server.Start() - go janitor.Run() go debugserver.Start() // Attempt to clean up after first shutdown signal diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go index 0e483a93a8308a..a76353d6161c5d 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go @@ -8,23 +8,11 @@ import ( "github.com/inconshreveable/log15" "github.com/pkg/errors" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/paths" - "github.com/sourcegraph/sourcegraph/internal/codeintel/lsifserver/client" ) -type PruneFn func(ctx context.Context) (int64, bool, error) - -func defaultPruneFn(ctx context.Context) (int64, bool, error) { - id, prunable, err := client.DefaultClient.Prune(ctx) - if err != nil { - return 0, false, errors.Wrap(err, "lsifserver.Prune") - } - - return id, prunable, nil -} - // freeSpace determines the space available on the device containing the bundle directory, // then calls cleanOldBundles to free enough space to get back below the disk usage threshold. -func (j *Janitor) freeSpace(pruneFn PruneFn) error { +func (j *Janitor) freeSpace() error { var fs syscall.Statfs_t if err := syscall.Statfs(j.bundleDir, &fs); err != nil { return err @@ -35,7 +23,7 @@ func (j *Janitor) freeSpace(pruneFn PruneFn) error { desiredFreeBytes := uint64(float64(diskSizeBytes) * float64(j.desiredPercentFree) / 100.0) if freeBytes < desiredFreeBytes { - return j.evictBundles(pruneFn, uint64(desiredFreeBytes-freeBytes)) + return j.evictBundles(uint64(desiredFreeBytes - freeBytes)) } return nil @@ -43,9 +31,9 @@ func (j *Janitor) freeSpace(pruneFn PruneFn) error { // evictBundles removes bundles from the database (via precise-code-intel-api-server) // and the filesystem until at least bytesToFree, or there are no more prunable bundles. -func (j *Janitor) evictBundles(pruneFn func(ctx context.Context) (int64, bool, error), bytesToFree uint64) error { +func (j *Janitor) evictBundles(bytesToFree uint64) error { for bytesToFree > 0 { - bytesRemoved, pruned, err := j.evictBundle(pruneFn) + bytesRemoved, pruned, err := j.evictBundle() if err != nil { return err } @@ -67,13 +55,16 @@ func (j *Janitor) evictBundles(pruneFn func(ctx context.Context) (int64, bool, e // the oldest bundle to remove then deletes the associated file. This method // returns the size of the deleted file on success, and returns a false-valued // flag if there are no prunable bundles. -func (j *Janitor) evictBundle(pruneFn func(ctx context.Context) (int64, bool, error)) (uint64, bool, error) { - id, prunable, err := pruneFn(context.Background()) - if err != nil || !prunable { - return 0, false, err +func (j *Janitor) evictBundle() (uint64, bool, error) { + id, prunable, err := j.db.DeleteOldestDump(context.Background()) + if err != nil { + return 0, false, errors.Wrap(err, "db.DeleteOldestDump") + } + if !prunable { + return 0, false, nil } - path := paths.DBFilename(j.bundleDir, id) + path := paths.DBFilename(j.bundleDir, int64(id)) fileInfo, err := os.Stat(path) if err != nil { diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/janitor.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/janitor.go index 5bc2654b3b3604..612acd2bdcff1f 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/janitor.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/janitor.go @@ -6,9 +6,11 @@ import ( "github.com/inconshreveable/log15" "github.com/pkg/errors" + "github.com/sourcegraph/sourcegraph/internal/codeintel/db" ) type Janitor struct { + db db.DB bundleDir string desiredPercentFree int janitorInterval time.Duration @@ -19,6 +21,7 @@ type Janitor struct { } func New( + db db.DB, bundleDir string, desiredPercentFree int, janitorInterval time.Duration, @@ -26,6 +29,7 @@ func New( metrics JanitorMetrics, ) *Janitor { return &Janitor{ + db: db, bundleDir: bundleDir, desiredPercentFree: desiredPercentFree, janitorInterval: janitorInterval, @@ -35,8 +39,7 @@ func New( } } -// Run periodically performs a best-effort cleanup process. See the following methods -// for more specifics: removeOldUploadFiles, removeOrphanedBundleFiles, and freeSpace. +// Run periodically performs a best-effort cleanup process. func (j *Janitor) Run() { for { if err := j.run(); err != nil { @@ -66,13 +69,17 @@ func (j *Janitor) run() error { return errors.Wrap(err, "janitor.removeOldUploadFiles") } - if err := j.removeOrphanedBundleFiles(defaultStatesFn); err != nil { + if err := j.removeOrphanedBundleFiles(); err != nil { return errors.Wrap(err, "janitor.removeOrphanedBundleFiles") } - if err := j.freeSpace(defaultPruneFn); err != nil { + if err := j.freeSpace(); err != nil { return errors.Wrap(err, "janitor.freeSpace") } + if err := j.removeProcessedUploadsWithoutBundleFile(); err != nil { + return errors.Wrap(err, "janitor.removeProcessedUploadsWithoutBundle") + } + return nil } diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/metrics.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/metrics.go index d54c81241ffe16..dba6fc13a1454b 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/metrics.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/metrics.go @@ -6,6 +6,7 @@ type JanitorMetrics struct { UploadFilesRemoved prometheus.Counter OrphanedBundleFilesRemoved prometheus.Counter EvictedBundleFilesRemoved prometheus.Counter + UploadRecordsRemoved prometheus.Counter Errors prometheus.Counter } @@ -28,6 +29,12 @@ func NewJanitorMetrics(r prometheus.Registerer) JanitorMetrics { }) r.MustRegister(evictedBundleFilesRemoved) + uploadRecordsRemoved := prometheus.NewCounter(prometheus.CounterOpts{ + Name: "src_bundle_manager_janitor_upload_records_removed_total", + Help: "Total number of processed upload records removed (with no corresponding bundle file)", + }) + r.MustRegister(uploadRecordsRemoved) + errors := prometheus.NewCounter(prometheus.CounterOpts{ Name: "src_bundle_manager_janitor_errors_total", Help: "Total number of errors when running the janitor", @@ -38,6 +45,7 @@ func NewJanitorMetrics(r prometheus.Registerer) JanitorMetrics { UploadFilesRemoved: uploadFilesRemoved, OrphanedBundleFilesRemoved: orphanedBundleFilesRemoved, EvictedBundleFilesRemoved: evictedBundleFilesRemoved, + UploadRecordsRemoved: uploadRecordsRemoved, Errors: errors, } } diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go index 9e27a45fe4ce93..3431dd5c295d75 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go @@ -11,28 +11,16 @@ import ( "github.com/inconshreveable/log15" "github.com/pkg/errors" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/paths" - "github.com/sourcegraph/sourcegraph/internal/codeintel/lsifserver/client" ) // OrphanedBundleBatchSize is the maximum number of bundle ids to request at // once from the precise-code-intel-api-server. const OrphanedBundleBatchSize = 100 -type StatesFn func(ctx context.Context, ids []int) (map[int]string, error) - -func defaultStatesFn(ctx context.Context, ids []int) (map[int]string, error) { - states, err := client.DefaultClient.States(ctx, ids) - if err != nil { - return nil, errors.Wrap(err, "lsifserver.States") - } - - return states, nil -} - // removeOrphanedBundleFiles calls the precise-code-intel-api-server to get the // current state of the bundle known by this bundle manager. Any bundle on disk // that is in an errored state or is unknown by the API is removed. -func (j *Janitor) removeOrphanedBundleFiles(statesFn StatesFn) error { +func (j *Janitor) removeOrphanedBundleFiles() error { pathsByID, err := j.databasePathsByID() if err != nil { return err @@ -45,9 +33,9 @@ func (j *Janitor) removeOrphanedBundleFiles(statesFn StatesFn) error { allStates := map[int]string{} for _, batch := range batchIntSlice(ids, OrphanedBundleBatchSize) { - states, err := statesFn(context.Background(), batch) + states, err := j.db.GetStates(context.Background(), batch) if err != nil { - return err + return errors.Wrap(err, "db.GetStates") } for k, v := range states { diff --git a/cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file.go similarity index 57% rename from cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file.go index f379efa19288ec..dc4d57a6b52bbb 100644 --- a/cmd/precise-code-intel-api-server/internal/janitor/remove_processed_uploads_without_bundle_file.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file.go @@ -5,36 +5,27 @@ import ( "github.com/inconshreveable/log15" "github.com/pkg/errors" + "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/paths" "github.com/sourcegraph/sourcegraph/internal/codeintel/gitserver" ) -// BundleBatchSize is the maximum number of bundle ids to request at -// once from the precise-code-intel-bundle-manager. -const BundleBatchSize = 100 - // removeProcessedUploadsWithoutBundleFile removes all processed upload records // that do not have a corresponding bundle file on disk. func (j *Janitor) removeProcessedUploadsWithoutBundleFile() error { ctx := context.Background() + // TODO(efritz) - paginate ids, err := j.db.GetDumpIDs(ctx) if err != nil { return errors.Wrap(err, "db.GetDumpIDs") } - allExists := map[int]bool{} - for _, batch := range batchIntSlice(ids, BundleBatchSize) { - exists, err := j.bundleManagerClient.Exists(ctx, batch) + for _, id := range ids { + exists, err := paths.PathExists(paths.DBFilename(j.bundleDir, int64(id))) if err != nil { - return errors.Wrap(err, "bundleManagerClient.Exists") + return errors.Wrap(err, "paths.PathExists") } - for k, v := range exists { - allExists[k] = v - } - } - - for id, exists := range allExists { if exists { continue } @@ -59,18 +50,3 @@ func (j *Janitor) removeProcessedUploadsWithoutBundleFile() error { return nil } - -// batchIntSlice returns slices of s (in order) at most batchSize in length. -func batchIntSlice(s []int, batchSize int) [][]int { - batches := [][]int{} - for len(s) > batchSize { - batches = append(batches, s[:batchSize]) - s = s[batchSize:] - } - - if len(s) > 0 { - batches = append(batches, s) - } - - return batches -} diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file_test.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file_test.go new file mode 100644 index 00000000000000..f78aebea630434 --- /dev/null +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file_test.go @@ -0,0 +1,52 @@ +package janitor + +import ( + "fmt" + "path/filepath" + "sort" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + dbmocks "github.com/sourcegraph/sourcegraph/internal/codeintel/db/mocks" + "github.com/sourcegraph/sourcegraph/internal/metrics" +) + +func TestRemoveProcessedUploadsWithoutBundleFile(t *testing.T) { + bundleDir := testRoot(t) + ids := []int{1, 2, 3, 4, 5} + + for _, id := range []int{1, 3, 5} { + path := filepath.Join(bundleDir, "dbs", fmt.Sprintf("%d.lsif.db", id)) + if err := makeFile(path, time.Now().Local()); err != nil { + t.Fatalf("unexpected error creating file %s: %s", path, err) + } + } + + mockDB := dbmocks.NewMockDB() + mockDB.GetDumpIDsFunc.SetDefaultReturn(ids, nil) + + j := &Janitor{ + db: mockDB, + bundleDir: bundleDir, + metrics: NewJanitorMetrics(metrics.TestRegisterer), + } + + if err := j.removeProcessedUploadsWithoutBundleFile(); err != nil { + t.Fatalf("unexpected error removing processed uploads without bundle files: %s", err) + } + + if len(mockDB.DeleteUploadByIDFunc.History()) != 2 { + t.Errorf("unexpected number of DeleteUploadByID calls. want=%d have=%d", 2, len(mockDB.DeleteUploadByIDFunc.History())) + } else { + ids := []int{ + mockDB.DeleteUploadByIDFunc.History()[0].Arg1, + mockDB.DeleteUploadByIDFunc.History()[1].Arg1, + } + sort.Ints(ids) + + if diff := cmp.Diff([]int{2, 4}, ids); diff != "" { + t.Errorf("unexpected dump ids (-want +got):\n%s", diff) + } + } +} diff --git a/cmd/precise-code-intel-bundle-manager/internal/paths/exists.go b/cmd/precise-code-intel-bundle-manager/internal/paths/exists.go new file mode 100644 index 00000000000000..2b93b9d9799e26 --- /dev/null +++ b/cmd/precise-code-intel-bundle-manager/internal/paths/exists.go @@ -0,0 +1,15 @@ +package paths + +import "os" + +func PathExists(filename string) (bool, error) { + if _, err := os.Stat(filename); err != nil { + if os.IsNotExist(err) { + return false, nil + } + + return false, err + } + + return true, nil +} diff --git a/cmd/precise-code-intel-bundle-manager/internal/server/handler.go b/cmd/precise-code-intel-bundle-manager/internal/server/handler.go index 73f077094d2ebb..cff875da6f9a83 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/server/handler.go +++ b/cmd/precise-code-intel-bundle-manager/internal/server/handler.go @@ -37,7 +37,6 @@ func (s *Server) handler() http.Handler { mux.Path("/dbs/{id:[0-9]+}/monikersByPosition").Methods("GET").HandlerFunc(s.handleMonikersByPosition) mux.Path("/dbs/{id:[0-9]+}/monikerResults").Methods("GET").HandlerFunc(s.handleMonikerResults) mux.Path("/dbs/{id:[0-9]+}/packageInformation").Methods("GET").HandlerFunc(s.handlePackageInformation) - mux.Path("/exists").Methods("GET").HandlerFunc(s.handleBulkExists) mux.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK) }) @@ -206,23 +205,6 @@ func (s *Server) handlePackageInformation(w http.ResponseWriter, r *http.Request }) } -// GET /exists?ids=1,2,3 -func (s *Server) handleBulkExists(w http.ResponseWriter, r *http.Request) { - existsMap := map[int]bool{} - for _, id := range getQueryInts(r, "ids") { - exists, err := fileExists(paths.DBFilename(s.bundleDir, int64(id))) - if err != nil { - log15.Error("Failed to check filepath", "err", err) - http.Error(w, fmt.Sprintf("failed to check filepath: %s", err.Error()), http.StatusInternalServerError) - return - } - - existsMap[id] = exists - } - - writeJSON(w, existsMap) -} - // doUpload writes the HTTP request body to the path determined by the given // makeFilename function. func (s *Server) doUpload(w http.ResponseWriter, r *http.Request, makeFilename func(bundleDir string, id int64) string) bool { @@ -293,7 +275,7 @@ func (s *Server) dbQueryErr(w http.ResponseWriter, r *http.Request, handler dbQu cached = false // Ensure database exists prior to opening - if exists, err := fileExists(filename); err != nil { + if exists, err := paths.PathExists(filename); err != nil { return nil, err } else if !exists { return nil, ErrUnknownDatabase @@ -308,7 +290,7 @@ func (s *Server) dbQueryErr(w http.ResponseWriter, r *http.Request, handler dbQu // the DB file was deleted between the exists check and opening the database // and SQLite has created a new, empty database that is not yet written to disk. // Ensure database exists prior to opening - if exists, err := fileExists(filename); err != nil { + if exists, err := paths.PathExists(filename); err != nil { return nil, err } else if !exists { sqliteReader.Close() diff --git a/cmd/precise-code-intel-bundle-manager/internal/server/util.go b/cmd/precise-code-intel-bundle-manager/internal/server/util.go index 4c683d9e68628a..a2f71ca11084a9 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/server/util.go +++ b/cmd/precise-code-intel-bundle-manager/internal/server/util.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "net/http" - "os" "strconv" "strings" @@ -72,15 +71,3 @@ func writeJSON(w http.ResponseWriter, payload interface{}) { copyAll(w, bytes.NewReader(data)) } - -func fileExists(filename string) (bool, error) { - if _, err := os.Stat(filename); err != nil { - if os.IsNotExist(err) { - return false, nil - } - - return false, err - } - - return true, nil -} diff --git a/cmd/precise-code-intel-bundle-manager/main.go b/cmd/precise-code-intel-bundle-manager/main.go index 4d063b67949670..3bf85d0a1ba341 100644 --- a/cmd/precise-code-intel-bundle-manager/main.go +++ b/cmd/precise-code-intel-bundle-manager/main.go @@ -14,6 +14,8 @@ import ( "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/janitor" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/paths" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/server" + "github.com/sourcegraph/sourcegraph/internal/codeintel/db" + "github.com/sourcegraph/sourcegraph/internal/conf" "github.com/sourcegraph/sourcegraph/internal/debugserver" "github.com/sourcegraph/sourcegraph/internal/env" "github.com/sourcegraph/sourcegraph/internal/metrics" @@ -57,10 +59,11 @@ func main() { resultChunkCacheSize, ) + db := db.NewObserved(mustInitializeDatabase(), observationContext) metrics.MustRegisterDiskMonitor(bundleDir) - janitorMetrics := janitor.NewJanitorMetrics(prometheus.DefaultRegisterer) server := server.New(bundleDir, databaseCache, documentCache, resultChunkCache, observationContext) - janitor := janitor.New(bundleDir, desiredPercentFree, janitorInterval, maxUploadAge, janitorMetrics) + janitorMetrics := janitor.NewJanitorMetrics(prometheus.DefaultRegisterer) + janitor := janitor.New(db, bundleDir, desiredPercentFree, janitorInterval, maxUploadAge, janitorMetrics) go server.Start() go janitor.Run() @@ -107,3 +110,19 @@ func prepCaches(r prometheus.Registerer, databaseCacheSize, documentCacheSize, r return databaseCache, documentCache, resultChunkCache } + +func mustInitializeDatabase() db.DB { + postgresDSN := conf.Get().ServiceConnections.PostgresDSN + conf.Watch(func() { + if newDSN := conf.Get().ServiceConnections.PostgresDSN; postgresDSN != newDSN { + log.Fatalf("detected repository DSN change, restarting to take effect: %s", newDSN) + } + }) + + db, err := db.New(postgresDSN) + if err != nil { + log.Fatalf("failed to initialize db store: %s", err) + } + + return db +} diff --git a/dev/check/go-dbconn-import.sh b/dev/check/go-dbconn-import.sh index 4439bd5c16bf43..98d9d15f15fcf0 100755 --- a/dev/check/go-dbconn-import.sh +++ b/dev/check/go-dbconn-import.sh @@ -7,7 +7,7 @@ echo "--- go dbconn import" set -euf -o pipefail -allowed='^github.com/sourcegraph/sourcegraph/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/repo-updater|github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server' +allowed='^github.com/sourcegraph/sourcegraph/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/repo-updater' # shellcheck disable=SC2016 template='{{with $pkg := .}}{{ range $pkg.Deps }}{{ printf "%s imports %s\n" $pkg.ImportPath .}}{{end}}{{end}}' diff --git a/internal/codeintel/lsifserver/client/internal.go b/internal/codeintel/lsifserver/client/internal.go deleted file mode 100644 index 1a1b964d5d75e0..00000000000000 --- a/internal/codeintel/lsifserver/client/internal.go +++ /dev/null @@ -1,61 +0,0 @@ -package client - -import ( - "bytes" - "context" - "encoding/json" - "io/ioutil" -) - -func (c *Client) Prune(ctx context.Context) (int64, bool, error) { - req := &lsifRequest{ - method: "POST", - path: "/prune", - } - - var payload struct { - ID *int64 `json:"id"` - } - if _, err := c.do(ctx, req, &payload); err != nil { - return 0, false, err - } - - if payload.ID == nil { - return 0, false, nil - } - return *payload.ID, true, nil -} - -func (c *Client) States(ctx context.Context, ids []int) (map[int]string, error) { - serialized, err := json.Marshal(map[string]interface{}{"ids": ids}) - if err != nil { - return nil, err - } - - req := &lsifRequest{ - method: "POST", - path: "/uploads", - body: ioutil.NopCloser(bytes.NewReader(serialized)), - } - - var payload struct { - Value []json.RawMessage `json:"value"` - } - if _, err := c.do(ctx, req, &payload); err != nil { - return nil, err - } - - states := map[int]string{} - for _, pair := range payload.Value { - var key int - var value string - payload := []interface{}{&key, &value} - if err := json.Unmarshal([]byte(pair), &payload); err != nil { - return nil, err - } - - states[key] = value - } - - return states, nil -} diff --git a/monitoring/precise_code_intel_api_server.go b/monitoring/precise_code_intel_api_server.go index ace2836118ba1d..bb90c6785c00c8 100644 --- a/monitoring/precise_code_intel_api_server.go +++ b/monitoring/precise_code_intel_api_server.go @@ -55,32 +55,6 @@ func PreciseCodeIntelAPIServer() *Container { }, }, }, - { - Title: "Janitor - cleans up upload records in PostgreSQL", - Hidden: true, - Rows: []Row{ - { - { - Name: "janitor_errors", - Description: "janitor errors every 5m", - Query: `sum(increase(src_api_server_janitor_errors_total[5m]))`, - DataMayNotExist: true, - Warning: Alert{GreaterOrEqual: 20}, - PanelOptions: PanelOptions().LegendFormat("errors"), - PossibleSolutions: "none", - }, - { - Name: "janitor_uploads_without_bundle_files", - Description: "upload records removed (due to missing bundle files) every 5m", - Query: `sum(increase(src_api_server_janitor_upload_records_removed_total[5m]))`, - DataMayNotExist: true, - Warning: Alert{GreaterOrEqual: 20}, - PanelOptions: PanelOptions().LegendFormat("records removed"), - PossibleSolutions: "none", - }, - }, - }, - }, { Title: "Internal service requests", Hidden: true, diff --git a/monitoring/precise_code_intel_bundle_manager.go b/monitoring/precise_code_intel_bundle_manager.go index 3f722f90d40d56..876360b79c84fb 100644 --- a/monitoring/precise_code_intel_bundle_manager.go +++ b/monitoring/precise_code_intel_bundle_manager.go @@ -92,6 +92,9 @@ func PreciseCodeIntelBundleManager() *Container { PanelOptions: PanelOptions().LegendFormat("files removed"), PossibleSolutions: "none", }, + }, + // TODO - reorganize this + { { Name: "janitor_orphaned_dumps", Description: "bundle files removed (with no corresponding database entry) every 5m", @@ -110,6 +113,15 @@ func PreciseCodeIntelBundleManager() *Container { PanelOptions: PanelOptions().LegendFormat("files removed"), PossibleSolutions: "none", }, + { + Name: "janitor_uploads_without_bundle_files", + Description: "upload records removed (due to missing bundle files) every 5m", + Query: `sum(increase(src_bundle_manager_janitor_upload_records_removed_total[5m]))`, + DataMayNotExist: true, + Warning: Alert{GreaterOrEqual: 20}, + PanelOptions: PanelOptions().LegendFormat("records removed"), + PossibleSolutions: "none", + }, }, }, }, From 4c11e9fec06bc1cfae5b60d069bda59e53709855 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Tue, 19 May 2020 20:24:04 -0500 Subject: [PATCH 02/11] Fix tests. --- .../internal/janitor/free_space_test.go | 30 ++++++++----- .../remove_orphaned_bundle_files_test.go | 45 ++++++++++--------- 2 files changed, 44 insertions(+), 31 deletions(-) diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space_test.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space_test.go index e8526db4ff3cb3..7dd94a57ba02d7 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space_test.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + dbmocks "github.com/sourcegraph/sourcegraph/internal/codeintel/db/mocks" "github.com/sourcegraph/sourcegraph/internal/metrics" ) @@ -33,17 +34,19 @@ func TestEvictBundlesStopsAfterFreeingDesiredSpace(t *testing.T) { } calls := 0 - pruneFn := func(ctx context.Context) (int64, bool, error) { + mockDB := dbmocks.NewMockDB() + mockDB.DeleteOldestDumpFunc.SetDefaultHook(func(ctx context.Context) (int, bool, error) { calls++ - return int64(calls), true, nil - } + return calls, true, nil + }) j := &Janitor{ + db: mockDB, bundleDir: bundleDir, metrics: NewJanitorMetrics(metrics.TestRegisterer), } - if err := j.evictBundles(pruneFn, 100); err != nil { + if err := j.evictBundles(100); err != nil { t.Fatalf("unexpected error evicting bundles: %s", err) } @@ -81,22 +84,25 @@ func TestEvictBundlesStopsWithNoPrunableDatabases(t *testing.T) { } idsToPrune := []int{1, 2, 3, 4, 5} - pruneFn := func(ctx context.Context) (int64, bool, error) { + + mockDB := dbmocks.NewMockDB() + mockDB.DeleteOldestDumpFunc.SetDefaultHook(func(ctx context.Context) (int, bool, error) { if len(idsToPrune) == 0 { return 0, false, nil } id := idsToPrune[0] idsToPrune = idsToPrune[1:] - return int64(id), true, nil - } + return id, true, nil + }) j := &Janitor{ + db: mockDB, bundleDir: bundleDir, metrics: NewJanitorMetrics(metrics.TestRegisterer), } - if err := j.evictBundles(pruneFn, 100); err != nil { + if err := j.evictBundles(100); err != nil { t.Fatalf("unexpected error evicting bundles: %s", err) } @@ -115,20 +121,22 @@ func TestEvictBundlesNoBundleFile(t *testing.T) { bundleDir := testRoot(t) called := false - pruneFn := func(ctx context.Context) (int64, bool, error) { + mockDB := dbmocks.NewMockDB() + mockDB.DeleteOldestDumpFunc.SetDefaultHook(func(ctx context.Context) (int, bool, error) { if !called { called = true return 42, true, nil } return 0, false, nil - } + }) j := &Janitor{ + db: mockDB, bundleDir: bundleDir, metrics: NewJanitorMetrics(metrics.TestRegisterer), } - if err := j.evictBundles(pruneFn, 100); err != nil { + if err := j.evictBundles(100); err != nil { t.Fatalf("unexpected error evicting bundles: %s", err) } } diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go index 76a8679effd3f1..e08d3593806654 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + dbmocks "github.com/sourcegraph/sourcegraph/internal/codeintel/db/mocks" "github.com/sourcegraph/sourcegraph/internal/metrics" ) @@ -23,15 +24,11 @@ func TestRemoveOrphanedBundleFile(t *testing.T) { } } - j := &Janitor{ - bundleDir: bundleDir, - metrics: NewJanitorMetrics(metrics.TestRegisterer), - } - var idArgs [][]int - statesFn := func(ctx context.Context, args []int) (map[int]string, error) { - sort.Ints(args) - idArgs = append(idArgs, args) + mockDB := dbmocks.NewMockDB() + mockDB.GetStatesFunc.SetDefaultHook(func(ctx context.Context, ids []int) (map[int]string, error) { + sort.Ints(ids) + idArgs = append(idArgs, ids) return map[int]string{ 1: "completed", @@ -42,9 +39,15 @@ func TestRemoveOrphanedBundleFile(t *testing.T) { 9: "errored", 10: "errored", }, nil + }) + + j := &Janitor{ + db: mockDB, + bundleDir: bundleDir, + metrics: NewJanitorMetrics(metrics.TestRegisterer), } - if err := j.removeOrphanedBundleFiles(statesFn); err != nil { + if err := j.removeOrphanedBundleFiles(); err != nil { t.Fatalf("unexpected error removing orphaned bundle files: %s", err) } @@ -78,25 +81,27 @@ func TestRemoveOrphanedBundleFilesMaxRequestBatchSize(t *testing.T) { } } - j := &Janitor{ - bundleDir: bundleDir, - metrics: NewJanitorMetrics(metrics.TestRegisterer), - } - var idArgs [][]int - statesFn := func(ctx context.Context, args []int) (map[int]string, error) { - idArgs = append(idArgs, args) + mockDB := dbmocks.NewMockDB() + mockDB.GetStatesFunc.SetDefaultHook(func(ctx context.Context, ids []int) (map[int]string, error) { + idArgs = append(idArgs, ids) states := map[int]string{} - for _, arg := range args { - if arg%2 == 0 { - states[arg] = "completed" + for _, id := range ids { + if id%2 == 0 { + states[id] = "completed" } } return states, nil + }) + + j := &Janitor{ + db: mockDB, + bundleDir: bundleDir, + metrics: NewJanitorMetrics(metrics.TestRegisterer), } - if err := j.removeOrphanedBundleFiles(statesFn); err != nil { + if err := j.removeOrphanedBundleFiles(); err != nil { t.Fatalf("unexpected error removing dead dumps: %s", err) } From 6293b44d0a2320ff33782ebd4d20a95fc228cf12 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Tue, 19 May 2020 20:29:52 -0500 Subject: [PATCH 03/11] Refactor janitor. --- ...t_bundle_file.go => bundleless_uploads.go} | 2 +- ...ile_test.go => bundleless_uploads_test.go} | 0 .../internal/janitor/free_space.go | 10 +++++----- ...ld_upload_files.go => old_upload_files.go} | 0 ...files_test.go => old_upload_files_test.go} | 0 ...ndle_files.go => orphaned_bundle_files.go} | 14 +++++++------- ..._test.go => orphaned_bundle_files_test.go} | 19 +++++++++---------- 7 files changed, 22 insertions(+), 23 deletions(-) rename cmd/precise-code-intel-bundle-manager/internal/janitor/{remove_processed_uploads_without_bundle_file.go => bundleless_uploads.go} (97%) rename cmd/precise-code-intel-bundle-manager/internal/janitor/{remove_processed_uploads_without_bundle_file_test.go => bundleless_uploads_test.go} (100%) rename cmd/precise-code-intel-bundle-manager/internal/janitor/{remove_old_upload_files.go => old_upload_files.go} (100%) rename cmd/precise-code-intel-bundle-manager/internal/janitor/{remove_old_upload_files_test.go => old_upload_files_test.go} (100%) rename cmd/precise-code-intel-bundle-manager/internal/janitor/{remove_orphaned_bundle_files.go => orphaned_bundle_files.go} (87%) rename cmd/precise-code-intel-bundle-manager/internal/janitor/{remove_orphaned_bundle_files_test.go => orphaned_bundle_files_test.go} (92%) diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/bundleless_uploads.go similarity index 97% rename from cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/bundleless_uploads.go index dc4d57a6b52bbb..49b1f7c50eb9f4 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/bundleless_uploads.go @@ -14,7 +14,7 @@ import ( func (j *Janitor) removeProcessedUploadsWithoutBundleFile() error { ctx := context.Background() - // TODO(efritz) - paginate + // TODO(efritz) - request in batches ids, err := j.db.GetDumpIDs(ctx) if err != nil { return errors.Wrap(err, "db.GetDumpIDs") diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file_test.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/bundleless_uploads_test.go similarity index 100% rename from cmd/precise-code-intel-bundle-manager/internal/janitor/remove_processed_uploads_without_bundle_file_test.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/bundleless_uploads_test.go diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go index a76353d6161c5d..ac442a5e8c007f 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/free_space.go @@ -29,8 +29,9 @@ func (j *Janitor) freeSpace() error { return nil } -// evictBundles removes bundles from the database (via precise-code-intel-api-server) -// and the filesystem until at least bytesToFree, or there are no more prunable bundles. +// evictBundles removes completed upload recors from the database and then deletes the +// associated bundle file from the filesystem until at least bytesToFree, or there are +// no more prunable bundles. func (j *Janitor) evictBundles(bytesToFree uint64) error { for bytesToFree > 0 { bytesRemoved, pruned, err := j.evictBundle() @@ -51,9 +52,8 @@ func (j *Janitor) evictBundles(bytesToFree uint64) error { return nil } -// evictBundle calls the precise-code-intel-api-server for the identifier of -// the oldest bundle to remove then deletes the associated file. This method -// returns the size of the deleted file on success, and returns a false-valued +// evictBundle removes the oldest bundle from the database, then deletes the associated file. +// This method returns the size of the deleted file on success, and returns a false-valued // flag if there are no prunable bundles. func (j *Janitor) evictBundle() (uint64, bool, error) { id, prunable, err := j.db.DeleteOldestDump(context.Background()) diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_old_upload_files.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/old_upload_files.go similarity index 100% rename from cmd/precise-code-intel-bundle-manager/internal/janitor/remove_old_upload_files.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/old_upload_files.go diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_old_upload_files_test.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/old_upload_files_test.go similarity index 100% rename from cmd/precise-code-intel-bundle-manager/internal/janitor/remove_old_upload_files_test.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/old_upload_files_test.go diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go similarity index 87% rename from cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go index 3431dd5c295d75..67c14f955ba18d 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go @@ -13,8 +13,8 @@ import ( "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-bundle-manager/internal/paths" ) -// OrphanedBundleBatchSize is the maximum number of bundle ids to request at -// once from the precise-code-intel-api-server. +// OrphanedBundleBatchSize is the maximum number of bundle ids to request the state +// of from the database at once. const OrphanedBundleBatchSize = 100 // removeOrphanedBundleFiles calls the precise-code-intel-api-server to get the @@ -31,20 +31,20 @@ func (j *Janitor) removeOrphanedBundleFiles() error { ids = append(ids, id) } - allStates := map[int]string{} + states := map[int]string{} for _, batch := range batchIntSlice(ids, OrphanedBundleBatchSize) { - states, err := j.db.GetStates(context.Background(), batch) + batchStates, err := j.db.GetStates(context.Background(), batch) if err != nil { return errors.Wrap(err, "db.GetStates") } - for k, v := range states { - allStates[k] = v + for k, v := range batchStates { + states[k] = v } } for id, path := range pathsByID { - if state, exists := allStates[id]; !exists || state == "errored" { + if state, exists := states[id]; !exists || state == "errored" { if err := os.Remove(path); err != nil { return err } diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files_test.go similarity index 92% rename from cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go rename to cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files_test.go index e08d3593806654..f644b7e486a1df 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/remove_orphaned_bundle_files_test.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files_test.go @@ -24,12 +24,9 @@ func TestRemoveOrphanedBundleFile(t *testing.T) { } } - var idArgs [][]int mockDB := dbmocks.NewMockDB() mockDB.GetStatesFunc.SetDefaultHook(func(ctx context.Context, ids []int) (map[int]string, error) { sort.Ints(ids) - idArgs = append(idArgs, ids) - return map[int]string{ 1: "completed", 2: "queued", @@ -61,6 +58,11 @@ func TestRemoveOrphanedBundleFile(t *testing.T) { t.Errorf("unexpected directory contents (-want +got):\n%s", diff) } + var idArgs [][]int + for _, call := range mockDB.GetStatesFunc.History() { + idArgs = append(idArgs, call.Arg1) + } + expectedArgs := [][]int{ids} if diff := cmp.Diff(expectedArgs, idArgs); diff != "" { t.Errorf("unexpected arguments to statesFn (-want +got):\n%s", diff) @@ -81,11 +83,8 @@ func TestRemoveOrphanedBundleFilesMaxRequestBatchSize(t *testing.T) { } } - var idArgs [][]int mockDB := dbmocks.NewMockDB() mockDB.GetStatesFunc.SetDefaultHook(func(ctx context.Context, ids []int) (map[int]string, error) { - idArgs = append(idArgs, ids) - states := map[int]string{} for _, id := range ids { if id%2 == 0 { @@ -115,12 +114,12 @@ func TestRemoveOrphanedBundleFilesMaxRequestBatchSize(t *testing.T) { } var allArgs []int - for _, args := range idArgs { - if len(args) > OrphanedBundleBatchSize { - t.Errorf("unexpected large slice: want < %d have=%d", OrphanedBundleBatchSize, len(args)) + for _, call := range mockDB.GetStatesFunc.History() { + if len(call.Arg1) > OrphanedBundleBatchSize { + t.Errorf("unexpected large slice: want < %d have=%d", OrphanedBundleBatchSize, len(call.Arg1)) } - allArgs = append(allArgs, args...) + allArgs = append(allArgs, call.Arg1...) } sort.Ints(allArgs) From 9e5554af1d095b694658719d41a35f6deec19a4a Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Tue, 19 May 2020 20:30:55 -0500 Subject: [PATCH 04/11] Undo commit that doesn't actually do anaything. --- dev/check/go-dbconn-import.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/check/go-dbconn-import.sh b/dev/check/go-dbconn-import.sh index 98d9d15f15fcf0..4439bd5c16bf43 100755 --- a/dev/check/go-dbconn-import.sh +++ b/dev/check/go-dbconn-import.sh @@ -7,7 +7,7 @@ echo "--- go dbconn import" set -euf -o pipefail -allowed='^github.com/sourcegraph/sourcegraph/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/repo-updater' +allowed='^github.com/sourcegraph/sourcegraph/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/frontend|github.com/sourcegraph/sourcegraph/enterprise/cmd/repo-updater|github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server' # shellcheck disable=SC2016 template='{{with $pkg := .}}{{ range $pkg.Deps }}{{ printf "%s imports %s\n" $pkg.ImportPath .}}{{end}}{{end}}' From 71f876d777a3bc175f5ae3639ed9b349827414aa Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Tue, 19 May 2020 20:36:07 -0500 Subject: [PATCH 05/11] Update monitoring. --- .../precise_code_intel_bundle_manager.go | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/monitoring/precise_code_intel_bundle_manager.go b/monitoring/precise_code_intel_bundle_manager.go index 876360b79c84fb..3a236c7153f980 100644 --- a/monitoring/precise_code_intel_bundle_manager.go +++ b/monitoring/precise_code_intel_bundle_manager.go @@ -70,7 +70,7 @@ func PreciseCodeIntelBundleManager() *Container { }, }, { - Title: "Janitor - cleans up and keeps free space on disk", + Title: "Janitor - synchronizes database and filesystem and keeps free space on disk", Hidden: true, Rows: []Row{ { @@ -92,22 +92,21 @@ func PreciseCodeIntelBundleManager() *Container { PanelOptions: PanelOptions().LegendFormat("files removed"), PossibleSolutions: "none", }, - }, - // TODO - reorganize this - { { - Name: "janitor_orphaned_dumps", - Description: "bundle files removed (with no corresponding database entry) every 5m", - Query: `sum(increase(src_bundle_manager_janitor_orphaned_bundle_files_removed_total[5m]))`, + Name: "janitor_old_dumps", + Description: "bundle files removed (due to low disk space) every 5m", + Query: `sum(increase(src_bundle_manager_janitor_evicted_bundle_files_removed_total[5m]))`, DataMayNotExist: true, Warning: Alert{GreaterOrEqual: 20}, PanelOptions: PanelOptions().LegendFormat("files removed"), PossibleSolutions: "none", }, + }, + { { - Name: "janitor_old_dumps", - Description: "bundle files removed (after evicting them from the database) every 5m", - Query: `sum(increase(src_bundle_manager_janitor_evicted_bundle_files_removed_total[5m]))`, + Name: "janitor_orphaned_dumps", + Description: "bundle files removed (with no corresponding database entry) every 5m", + Query: `sum(increase(src_bundle_manager_janitor_orphaned_bundle_files_removed_total[5m]))`, DataMayNotExist: true, Warning: Alert{GreaterOrEqual: 20}, PanelOptions: PanelOptions().LegendFormat("files removed"), @@ -115,7 +114,7 @@ func PreciseCodeIntelBundleManager() *Container { }, { Name: "janitor_uploads_without_bundle_files", - Description: "upload records removed (due to missing bundle files) every 5m", + Description: "upload records removed (with no corresponding bundle file) every 5m", Query: `sum(increase(src_bundle_manager_janitor_upload_records_removed_total[5m]))`, DataMayNotExist: true, Warning: Alert{GreaterOrEqual: 20}, From f47c25d42c6b87672db8532108aa8f42c9667452 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Wed, 20 May 2020 10:05:47 -0500 Subject: [PATCH 06/11] Update comment. --- .../internal/janitor/orphaned_bundle_files.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go b/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go index 67c14f955ba18d..062a7d2c7a4eb7 100644 --- a/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go +++ b/cmd/precise-code-intel-bundle-manager/internal/janitor/orphaned_bundle_files.go @@ -17,9 +17,8 @@ import ( // of from the database at once. const OrphanedBundleBatchSize = 100 -// removeOrphanedBundleFiles calls the precise-code-intel-api-server to get the -// current state of the bundle known by this bundle manager. Any bundle on disk -// that is in an errored state or is unknown by the API is removed. +// removeOrphanedBundleFiles calls the removes any bundle on disk that is in an errored +// state or has no associated record in the database. func (j *Janitor) removeOrphanedBundleFiles() error { pathsByID, err := j.databasePathsByID() if err != nil { From 46d5981e59c947aa99fa9e63397ef33c6e2abf16 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Wed, 20 May 2020 10:45:45 -0500 Subject: [PATCH 07/11] Move api into internal/codeintel. --- cmd/precise-code-intel-api-server/internal/server/handler.go | 2 +- cmd/precise-code-intel-api-server/internal/server/locations.go | 2 +- cmd/precise-code-intel-api-server/internal/server/server.go | 2 +- cmd/precise-code-intel-api-server/main.go | 2 +- .../internal => internal/codeintel}/api/api.go | 0 .../internal => internal/codeintel}/api/api_test.go | 0 .../internal => internal/codeintel}/api/cursor.go | 0 .../internal => internal/codeintel}/api/cursor_test.go | 0 .../internal => internal/codeintel}/api/definitions.go | 0 .../internal => internal/codeintel}/api/definitions_test.go | 0 .../internal => internal/codeintel}/api/exists.go | 0 .../internal => internal/codeintel}/api/exists_test.go | 0 .../internal => internal/codeintel}/api/helpers_test.go | 0 .../internal => internal/codeintel}/api/hover.go | 0 .../internal => internal/codeintel}/api/hover_test.go | 0 .../internal => internal/codeintel}/api/locations.go | 0 .../internal => internal/codeintel}/api/monikers.go | 0 .../internal => internal/codeintel}/api/monikers_test.go | 0 .../internal => internal/codeintel}/api/observability.go | 0 .../internal => internal/codeintel}/api/references.go | 0 .../internal => internal/codeintel}/api/references_test.go | 0 21 files changed, 4 insertions(+), 4 deletions(-) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/api.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/api_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/cursor.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/cursor_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/definitions.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/definitions_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/exists.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/exists_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/helpers_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/hover.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/hover_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/locations.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/monikers.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/monikers_test.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/observability.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/references.go (100%) rename {cmd/precise-code-intel-api-server/internal => internal/codeintel}/api/references_test.go (100%) diff --git a/cmd/precise-code-intel-api-server/internal/server/handler.go b/cmd/precise-code-intel-api-server/internal/server/handler.go index ee57766b34b0a0..da784cd360939b 100644 --- a/cmd/precise-code-intel-api-server/internal/server/handler.go +++ b/cmd/precise-code-intel-api-server/internal/server/handler.go @@ -7,7 +7,7 @@ import ( "github.com/gorilla/mux" "github.com/inconshreveable/log15" "github.com/pkg/errors" - "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/api" + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" "github.com/sourcegraph/sourcegraph/internal/codeintel/gitserver" ) diff --git a/cmd/precise-code-intel-api-server/internal/server/locations.go b/cmd/precise-code-intel-api-server/internal/server/locations.go index 7588b7072e972f..a095dd99ec5a19 100644 --- a/cmd/precise-code-intel-api-server/internal/server/locations.go +++ b/cmd/precise-code-intel-api-server/internal/server/locations.go @@ -1,7 +1,7 @@ package server import ( - "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/api" + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" ) diff --git a/cmd/precise-code-intel-api-server/internal/server/server.go b/cmd/precise-code-intel-api-server/internal/server/server.go index 231a6a79919e3f..5c4a7638c22208 100644 --- a/cmd/precise-code-intel-api-server/internal/server/server.go +++ b/cmd/precise-code-intel-api-server/internal/server/server.go @@ -9,7 +9,7 @@ import ( "sync" "github.com/inconshreveable/log15" - "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/api" + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" "github.com/sourcegraph/sourcegraph/internal/codeintel/db" "github.com/sourcegraph/sourcegraph/internal/env" diff --git a/cmd/precise-code-intel-api-server/main.go b/cmd/precise-code-intel-api-server/main.go index 3d003907403565..11ba0f16ec9870 100644 --- a/cmd/precise-code-intel-api-server/main.go +++ b/cmd/precise-code-intel-api-server/main.go @@ -9,8 +9,8 @@ import ( "github.com/inconshreveable/log15" "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" - "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/api" "github.com/sourcegraph/sourcegraph/cmd/precise-code-intel-api-server/internal/server" + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" "github.com/sourcegraph/sourcegraph/internal/codeintel/db" "github.com/sourcegraph/sourcegraph/internal/codeintel/gitserver" diff --git a/cmd/precise-code-intel-api-server/internal/api/api.go b/internal/codeintel/api/api.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/api.go rename to internal/codeintel/api/api.go diff --git a/cmd/precise-code-intel-api-server/internal/api/api_test.go b/internal/codeintel/api/api_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/api_test.go rename to internal/codeintel/api/api_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/cursor.go b/internal/codeintel/api/cursor.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/cursor.go rename to internal/codeintel/api/cursor.go diff --git a/cmd/precise-code-intel-api-server/internal/api/cursor_test.go b/internal/codeintel/api/cursor_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/cursor_test.go rename to internal/codeintel/api/cursor_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/definitions.go b/internal/codeintel/api/definitions.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/definitions.go rename to internal/codeintel/api/definitions.go diff --git a/cmd/precise-code-intel-api-server/internal/api/definitions_test.go b/internal/codeintel/api/definitions_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/definitions_test.go rename to internal/codeintel/api/definitions_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/exists.go b/internal/codeintel/api/exists.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/exists.go rename to internal/codeintel/api/exists.go diff --git a/cmd/precise-code-intel-api-server/internal/api/exists_test.go b/internal/codeintel/api/exists_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/exists_test.go rename to internal/codeintel/api/exists_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/helpers_test.go b/internal/codeintel/api/helpers_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/helpers_test.go rename to internal/codeintel/api/helpers_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/hover.go b/internal/codeintel/api/hover.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/hover.go rename to internal/codeintel/api/hover.go diff --git a/cmd/precise-code-intel-api-server/internal/api/hover_test.go b/internal/codeintel/api/hover_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/hover_test.go rename to internal/codeintel/api/hover_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/locations.go b/internal/codeintel/api/locations.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/locations.go rename to internal/codeintel/api/locations.go diff --git a/cmd/precise-code-intel-api-server/internal/api/monikers.go b/internal/codeintel/api/monikers.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/monikers.go rename to internal/codeintel/api/monikers.go diff --git a/cmd/precise-code-intel-api-server/internal/api/monikers_test.go b/internal/codeintel/api/monikers_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/monikers_test.go rename to internal/codeintel/api/monikers_test.go diff --git a/cmd/precise-code-intel-api-server/internal/api/observability.go b/internal/codeintel/api/observability.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/observability.go rename to internal/codeintel/api/observability.go diff --git a/cmd/precise-code-intel-api-server/internal/api/references.go b/internal/codeintel/api/references.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/references.go rename to internal/codeintel/api/references.go diff --git a/cmd/precise-code-intel-api-server/internal/api/references_test.go b/internal/codeintel/api/references_test.go similarity index 100% rename from cmd/precise-code-intel-api-server/internal/api/references_test.go rename to internal/codeintel/api/references_test.go From 7c8309c1c344575cf2b29802048197559e9cba84 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Wed, 20 May 2020 11:50:06 -0500 Subject: [PATCH 08/11] Copy api-server/internal/server into internal/codeintel/lsifserver/client and call it directly. --- enterprise/cmd/frontend/main.go | 63 ++++- .../codeintel/lsifserver/proxy/proxy.go | 8 +- .../internal/codeintel/resolvers/query.go | 8 +- .../internal/codeintel/resolvers/resolver.go | 19 +- .../internal/codeintel/resolvers/upload.go | 4 +- internal/codeintel/db/db.go | 4 + .../codeintel/lsifserver/client/client.go | 23 +- .../codeintel/lsifserver/client/handler.go | 216 ++++++++++++++++++ .../codeintel/lsifserver/client/locations.go | 27 +++ internal/codeintel/lsifserver/client/proxy.go | 35 +++ .../codeintel/lsifserver/client/server.go | 25 ++ internal/codeintel/lsifserver/client/util.go | 94 ++++++++ 12 files changed, 496 insertions(+), 30 deletions(-) create mode 100644 internal/codeintel/lsifserver/client/handler.go create mode 100644 internal/codeintel/lsifserver/client/locations.go create mode 100644 internal/codeintel/lsifserver/client/server.go create mode 100644 internal/codeintel/lsifserver/client/util.go diff --git a/enterprise/cmd/frontend/main.go b/enterprise/cmd/frontend/main.go index 7b37af751322db..1ffcb1e6fdebc9 100644 --- a/enterprise/cmd/frontend/main.go +++ b/enterprise/cmd/frontend/main.go @@ -8,11 +8,15 @@ package main import ( "context" "database/sql" + "fmt" "log" "os" "strconv" "time" + "github.com/inconshreveable/log15" + "github.com/opentracing/opentracing-go" + "github.com/prometheus/client_golang/prometheus" "github.com/sourcegraph/sourcegraph/cmd/frontend/authz" "github.com/sourcegraph/sourcegraph/cmd/frontend/db" "github.com/sourcegraph/sourcegraph/cmd/frontend/graphqlbackend" @@ -29,21 +33,30 @@ import ( campaignsResolvers "github.com/sourcegraph/sourcegraph/enterprise/internal/campaigns/resolvers" "github.com/sourcegraph/sourcegraph/enterprise/internal/codeintel/lsifserver/proxy" codeIntelResolvers "github.com/sourcegraph/sourcegraph/enterprise/internal/codeintel/resolvers" + codeintelapi "github.com/sourcegraph/sourcegraph/internal/codeintel/api" + bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" + codeinteldb "github.com/sourcegraph/sourcegraph/internal/codeintel/db" + codeintelgitserver "github.com/sourcegraph/sourcegraph/internal/codeintel/gitserver" + lsifserverclient "github.com/sourcegraph/sourcegraph/internal/codeintel/lsifserver/client" "github.com/sourcegraph/sourcegraph/internal/conf" "github.com/sourcegraph/sourcegraph/internal/db/dbconn" "github.com/sourcegraph/sourcegraph/internal/db/globalstatedb" + "github.com/sourcegraph/sourcegraph/internal/env" + "github.com/sourcegraph/sourcegraph/internal/observation" + "github.com/sourcegraph/sourcegraph/internal/trace" ) func main() { - initLicensing() - initResolvers() - initLSIFEndpoints() - // Connect to the database. if err := shared.InitDB(); err != nil { log.Fatalf("FATAL: %v", err) } + initLicensing() + initAuthz() + initCampaigns() + initCodeIntel() + clock := func() time.Time { return time.Now().UTC().Truncate(time.Microsecond) } @@ -136,9 +149,7 @@ func initLicensing() { } } -func initResolvers() { - graphqlbackend.NewCampaignsResolver = campaignsResolvers.NewResolver - graphqlbackend.NewCodeIntelResolver = codeIntelResolvers.NewResolver +func initAuthz() { graphqlbackend.NewAuthzResolver = func() graphqlbackend.AuthzResolver { return authzResolvers.NewResolver(dbconn.Global, func() time.Time { return time.Now().UTC().Truncate(time.Microsecond) @@ -146,8 +157,42 @@ func initResolvers() { } } -func initLSIFEndpoints() { - httpapi.NewLSIFServerProxy = proxy.NewProxy +func initCampaigns() { + graphqlbackend.NewCampaignsResolver = campaignsResolvers.NewResolver + +} + +var bundleManagerURL = env.Get("PRECISE_CODE_INTEL_BUNDLE_MANAGER_URL", "", "HTTP address for internal LSIF bundle manager server.") + +func initCodeIntel() { + if bundleManagerURL == "" { + log.Fatalf("invalid value for PRECISE_CODE_INTEL_BUNDLE_MANAGER_URL: no value supplied") + } + + observationContext := &observation.Context{ + Logger: log15.Root(), + Tracer: &trace.Tracer{Tracer: opentracing.GlobalTracer()}, + Registerer: prometheus.DefaultRegisterer, + } + + fmt.Printf("Conn: %v\n", dbconn.Global) + + db := codeinteldb.NewObserved(codeinteldb.NewWithHandle(dbconn.Global), observationContext) + bundleManagerClient := bundles.New(bundleManagerURL) + + client := lsifserverclient.New( + db, + bundleManagerClient, + codeintelapi.New(db, bundleManagerClient, codeintelgitserver.DefaultClient), + ) + + graphqlbackend.NewCodeIntelResolver = func() graphqlbackend.CodeIntelResolver { + return codeIntelResolvers.NewResolver(client) + } + + httpapi.NewLSIFServerProxy = func() (*httpapi.LSIFServerProxy, error) { + return proxy.NewProxy(client) + } } type usersStore struct{} diff --git a/enterprise/internal/codeintel/lsifserver/proxy/proxy.go b/enterprise/internal/codeintel/lsifserver/proxy/proxy.go index 6eb1a12c9d182e..55f225e203637b 100644 --- a/enterprise/internal/codeintel/lsifserver/proxy/proxy.go +++ b/enterprise/internal/codeintel/lsifserver/proxy/proxy.go @@ -20,13 +20,13 @@ import ( "github.com/sourcegraph/sourcegraph/internal/gitserver" ) -func NewProxy() (*httpapi.LSIFServerProxy, error) { +func NewProxy(lsifserverClient *client.Client) (*httpapi.LSIFServerProxy, error) { return &httpapi.LSIFServerProxy{ - UploadHandler: http.HandlerFunc(uploadProxyHandler()), + UploadHandler: http.HandlerFunc(uploadProxyHandler(lsifserverClient)), }, nil } -func uploadProxyHandler() func(http.ResponseWriter, *http.Request) { +func uploadProxyHandler(lsifserverClient *client.Client) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { q := r.URL.Query() repoName := q.Get("repository") @@ -72,7 +72,7 @@ func uploadProxyHandler() func(http.ResponseWriter, *http.Request) { return } - proxyResp, err := client.DefaultClient.RawRequest(ctx, proxyReq) + proxyResp, err := lsifserverClient.RawRequest(ctx, proxyReq) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/enterprise/internal/codeintel/resolvers/query.go b/enterprise/internal/codeintel/resolvers/query.go index 67ae028fc54f70..581b4db0c146a8 100644 --- a/enterprise/internal/codeintel/resolvers/query.go +++ b/enterprise/internal/codeintel/resolvers/query.go @@ -13,6 +13,8 @@ import ( ) type lsifQueryResolver struct { + lsifserverClient *client.Client + repositoryResolver *graphqlbackend.RepositoryResolver // commit is the requested target commit commit api.CommitID @@ -50,7 +52,7 @@ func (r *lsifQueryResolver) Definitions(ctx context.Context, args *graphqlbacken UploadID: upload.ID, } - locations, _, err := client.DefaultClient.Definitions(ctx, opts) + locations, _, err := r.lsifserverClient.Definitions(ctx, opts) if err != nil { return nil, err } @@ -121,7 +123,7 @@ func (r *lsifQueryResolver) References(ctx context.Context, args *graphqlbackend continue } - locations, nextURL, err := client.DefaultClient.References(ctx, opts) + locations, nextURL, err := r.lsifserverClient.References(ctx, opts) if err != nil { return nil, err } @@ -155,7 +157,7 @@ func (r *lsifQueryResolver) Hover(ctx context.Context, args *graphqlbackend.LSIF continue } - text, lspRange, err := client.DefaultClient.Hover(ctx, &struct { + text, lspRange, err := r.lsifserverClient.Hover(ctx, &struct { RepoID api.RepoID Commit api.CommitID Path string diff --git a/enterprise/internal/codeintel/resolvers/resolver.go b/enterprise/internal/codeintel/resolvers/resolver.go index 0b956101417319..0782c7e7c07aae 100644 --- a/enterprise/internal/codeintel/resolvers/resolver.go +++ b/enterprise/internal/codeintel/resolvers/resolver.go @@ -11,12 +11,16 @@ import ( "github.com/sourcegraph/sourcegraph/internal/codeintel/lsifserver/client" ) -type Resolver struct{} +type Resolver struct { + lsifserverClient *client.Client +} var _ graphqlbackend.CodeIntelResolver = &Resolver{} -func NewResolver() graphqlbackend.CodeIntelResolver { - return &Resolver{} +func NewResolver(lsifserverClient *client.Client) graphqlbackend.CodeIntelResolver { + return &Resolver{ + lsifserverClient: lsifserverClient, + } } func (r *Resolver) LSIFUploadByID(ctx context.Context, id graphql.ID) (graphqlbackend.LSIFUploadResolver, error) { @@ -25,7 +29,7 @@ func (r *Resolver) LSIFUploadByID(ctx context.Context, id graphql.ID) (graphqlba return nil, err } - lsifUpload, err := client.DefaultClient.GetUpload(ctx, &struct { + lsifUpload, err := r.lsifserverClient.GetUpload(ctx, &struct { UploadID int64 }{ UploadID: uploadID, @@ -48,7 +52,7 @@ func (r *Resolver) DeleteLSIFUpload(ctx context.Context, id graphql.ID) (*graphq return nil, err } - err = client.DefaultClient.DeleteUpload(ctx, &struct { + err = r.lsifserverClient.DeleteUpload(ctx, &struct { UploadID int64 }{ UploadID: uploadID, @@ -87,11 +91,11 @@ func (r *Resolver) LSIFUploads(ctx context.Context, args *graphqlbackend.LSIFRep opt.NextURL = &nextURL } - return &lsifUploadConnectionResolver{opt: opt}, nil + return &lsifUploadConnectionResolver{lsifserverClient: r.lsifserverClient, opt: opt}, nil } func (r *Resolver) LSIF(ctx context.Context, args *graphqlbackend.LSIFQueryArgs) (graphqlbackend.LSIFQueryResolver, error) { - uploads, err := client.DefaultClient.Exists(ctx, &struct { + uploads, err := r.lsifserverClient.Exists(ctx, &struct { RepoID api.RepoID Commit api.CommitID Path string @@ -110,6 +114,7 @@ func (r *Resolver) LSIF(ctx context.Context, args *graphqlbackend.LSIFQueryArgs) } return &lsifQueryResolver{ + lsifserverClient: r.lsifserverClient, repositoryResolver: args.Repository, commit: args.Commit, path: args.Path, diff --git a/enterprise/internal/codeintel/resolvers/upload.go b/enterprise/internal/codeintel/resolvers/upload.go index 7bb2668b04aa50..fc4f26e2bd7f2f 100644 --- a/enterprise/internal/codeintel/resolvers/upload.go +++ b/enterprise/internal/codeintel/resolvers/upload.go @@ -107,6 +107,8 @@ type LSIFUploadsListOptions struct { } type lsifUploadConnectionResolver struct { + lsifserverClient *client.Client + opt LSIFUploadsListOptions // cache results because they are used by multiple fields @@ -166,7 +168,7 @@ func (r *lsifUploadConnectionResolver) compute(ctx context.Context) ([]*lsif.LSI return } - r.uploads, r.nextURL, r.totalCount, r.err = client.DefaultClient.GetUploads(ctx, &struct { + r.uploads, r.nextURL, r.totalCount, r.err = r.lsifserverClient.GetUploads(ctx, &struct { RepoID api.RepoID Query *string State *string diff --git a/internal/codeintel/db/db.go b/internal/codeintel/db/db.go index 9bbbcbc0a87d4c..a886c87efb4c07 100644 --- a/internal/codeintel/db/db.go +++ b/internal/codeintel/db/db.go @@ -152,6 +152,10 @@ func New(postgresDSN string) (DB, error) { return &dbImpl{db: db}, nil } +func NewWithHandle(db *sql.DB) DB { + return &dbImpl{db: db} +} + // query performs QueryContext on the underlying connection. func (db *dbImpl) query(ctx context.Context, query *sqlf.Query) (*sql.Rows, error) { return db.db.QueryContext(ctx, query.Query(sqlf.PostgresBindVar), query.Args()...) diff --git a/internal/codeintel/lsifserver/client/client.go b/internal/codeintel/lsifserver/client/client.go index cbf51ad4da7700..2701533e56d6ab 100644 --- a/internal/codeintel/lsifserver/client/client.go +++ b/internal/codeintel/lsifserver/client/client.go @@ -6,6 +6,9 @@ import ( "strings" "sync" + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" + bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" + "github.com/sourcegraph/sourcegraph/internal/codeintel/db" "github.com/sourcegraph/sourcegraph/internal/endpoint" "github.com/sourcegraph/sourcegraph/internal/env" "github.com/sourcegraph/sourcegraph/internal/trace/ot" @@ -16,19 +19,27 @@ var ( preciseCodeIntelAPIServerURLsOnce sync.Once preciseCodeIntelAPIServerURLs *endpoint.Map +) + +type Client struct { + endpoint *endpoint.Map + HTTPClient *http.Client + server *Server +} - DefaultClient = &Client{ +func New( + db db.DB, + bundleManagerClient bundles.BundleManagerClient, + codeIntelAPI api.CodeIntelAPI, +) *Client { + return &Client{ endpoint: LSIFURLs(), HTTPClient: &http.Client{ // ot.Transport will propagate opentracing spans Transport: &ot.Transport{}, }, + server: NewServer(db, bundleManagerClient, codeIntelAPI), } -) - -type Client struct { - endpoint *endpoint.Map - HTTPClient *http.Client } func LSIFURLs() *endpoint.Map { diff --git a/internal/codeintel/lsifserver/client/handler.go b/internal/codeintel/lsifserver/client/handler.go new file mode 100644 index 00000000000000..8ff4cd8ddd1091 --- /dev/null +++ b/internal/codeintel/lsifserver/client/handler.go @@ -0,0 +1,216 @@ +package client + +import ( + "fmt" + "net/http" + + "github.com/inconshreveable/log15" + "github.com/pkg/errors" + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" + "github.com/sourcegraph/sourcegraph/internal/codeintel/gitserver" +) + +const DefaultUploadPageSize = 50 +const DefaultReferencesPageSize = 100 + +// GET /uploads/{id:[0-9]+} +func (s *Server) handleGetUploadByID(w http.ResponseWriter, r *http.Request) { + upload, exists, err := s.db.GetUploadByID(r.Context(), int(idFromRequest(r))) + if err != nil { + log15.Error("Failed to retrieve upload", "error", err) + http.Error(w, fmt.Sprintf("failed to retrieve upload: %s", err.Error()), http.StatusInternalServerError) + return + } + if !exists { + http.Error(w, "upload not found", http.StatusNotFound) + return + } + + writeJSON(w, upload) +} + +// DELETE /uploads/{id:[0-9]+} +func (s *Server) handleDeleteUploadByID(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + exists, err := s.db.DeleteUploadByID(ctx, int(idFromRequest(r)), func(repositoryID int) (string, error) { + tipCommit, err := gitserver.Head(ctx, s.db, repositoryID) + if err != nil { + return "", errors.Wrap(err, "gitserver.Head") + } + return tipCommit, nil + }) + if err != nil { + log15.Error("Failed to delete upload", "error", err) + http.Error(w, fmt.Sprintf("failed to delete upload: %s", err.Error()), http.StatusInternalServerError) + return + } + if !exists { + http.Error(w, "upload not found", http.StatusNotFound) + return + } + + w.WriteHeader(http.StatusNoContent) +} + +// GET /uploads/repository/{id:[0-9]+} +func (s *Server) handleGetUploadsByRepo(w http.ResponseWriter, r *http.Request) { + id := int(idFromRequest(r)) + limit := getQueryIntDefault(r, "limit", DefaultUploadPageSize) + offset := getQueryInt(r, "offset") + + uploads, totalCount, err := s.db.GetUploadsByRepo( + r.Context(), + id, + getQuery(r, "state"), + getQuery(r, "query"), + getQueryBool(r, "visibleAtTip"), + limit, + offset, + ) + if err != nil { + log15.Error("Failed to list uploads", "error", err) + http.Error(w, fmt.Sprintf("failed to list uploads: %s", err.Error()), http.StatusInternalServerError) + return + } + + if offset+len(uploads) < totalCount { + w.Header().Set("Link", makeNextLink(r.URL, map[string]interface{}{ + "limit": limit, + "offset": offset + len(uploads), + })) + } + + writeJSON(w, map[string]interface{}{"uploads": uploads, "totalCount": totalCount}) +} + +// GET /exists +func (s *Server) handleExists(w http.ResponseWriter, r *http.Request) { + dumps, err := s.codeIntelAPI.FindClosestDumps( + r.Context(), + getQueryInt(r, "repositoryId"), + getQuery(r, "commit"), + getQuery(r, "path"), + ) + if err != nil { + log15.Error("Failed to handle exists request", "error", err) + http.Error(w, fmt.Sprintf("failed to handle exists request: %s", err.Error()), http.StatusInternalServerError) + return + } + + writeJSON(w, map[string]interface{}{"uploads": dumps}) +} + +// GET /definitions +func (s *Server) handleDefinitions(w http.ResponseWriter, r *http.Request) { + defs, err := s.codeIntelAPI.Definitions( + r.Context(), + getQuery(r, "path"), + getQueryInt(r, "line"), + getQueryInt(r, "character"), + getQueryInt(r, "uploadId"), + ) + if err != nil { + if err == api.ErrMissingDump { + http.Error(w, "no such dump", http.StatusNotFound) + return + } + + log15.Error("Failed to handle definitions request", "error", err) + http.Error(w, fmt.Sprintf("failed to handle definitions request: %s", err.Error()), http.StatusInternalServerError) + return + } + + outers, err := serializeLocations(defs) + if err != nil { + log15.Error("Failed to resolve locations", "error", err) + http.Error(w, fmt.Sprintf("failed to resolve locations: %s", err.Error()), http.StatusInternalServerError) + return + } + + writeJSON(w, map[string]interface{}{"locations": outers}) +} + +// GET /references +func (s *Server) handleReferences(w http.ResponseWriter, r *http.Request) { + cursor, err := api.DecodeOrCreateCursor( + getQuery(r, "path"), + getQueryInt(r, "line"), + getQueryInt(r, "character"), + getQueryInt(r, "uploadId"), + getQuery(r, "cursor"), + s.db, + s.bundleManagerClient, + ) + if err != nil { + if err == api.ErrMissingDump { + http.Error(w, "no such dump", http.StatusNotFound) + return + } + + log15.Error("Failed to prepare cursor", "error", err) + http.Error(w, fmt.Sprintf("failed to prepare cursor: %s", err.Error()), http.StatusInternalServerError) + return + } + + limit := getQueryIntDefault(r, "limit", DefaultReferencesPageSize) + if limit <= 0 { + http.Error(w, "illegal limit", http.StatusBadRequest) + return + } + + locations, newCursor, hasNewCursor, err := s.codeIntelAPI.References( + r.Context(), + getQueryInt(r, "repositoryId"), + getQuery(r, "commit"), + limit, + cursor, + ) + if err != nil { + log15.Error("Failed to handle references request", "error", err) + http.Error(w, fmt.Sprintf("failed to handle references request: %s", err.Error()), http.StatusInternalServerError) + return + } + + outers, err := serializeLocations(locations) + if err != nil { + log15.Error("Failed to resolve locations", "error", err) + http.Error(w, fmt.Sprintf("failed to resolve locations: %s", err.Error()), http.StatusInternalServerError) + return + } + + if hasNewCursor { + w.Header().Set("Link", makeNextLink(r.URL, map[string]interface{}{ + "cursor": api.EncodeCursor(newCursor), + })) + } + + writeJSON(w, map[string]interface{}{"locations": outers}) +} + +// GET /hover +func (s *Server) handleHover(w http.ResponseWriter, r *http.Request) { + text, rn, exists, err := s.codeIntelAPI.Hover( + r.Context(), + getQuery(r, "path"), + getQueryInt(r, "line"), + getQueryInt(r, "character"), + getQueryInt(r, "uploadId"), + ) + if err != nil { + if err == api.ErrMissingDump { + http.Error(w, "no such dump", http.StatusNotFound) + return + } + + log15.Error("Failed to handle hover request", "error", err) + http.Error(w, fmt.Sprintf("failed to handle hover request: %s", err.Error()), http.StatusInternalServerError) + return + } + + if !exists { + writeJSON(w, nil) + } else { + writeJSON(w, map[string]interface{}{"text": text, "range": rn}) + } +} diff --git a/internal/codeintel/lsifserver/client/locations.go b/internal/codeintel/lsifserver/client/locations.go new file mode 100644 index 00000000000000..1b26cb89d4b1ac --- /dev/null +++ b/internal/codeintel/lsifserver/client/locations.go @@ -0,0 +1,27 @@ +package client + +import ( + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" + bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" +) + +type APILocation struct { + RepositoryID int `json:"repositoryId"` + Commit string `json:"commit"` + Path string `json:"path"` + Range bundles.Range `json:"range"` +} + +func serializeLocations(resolvedLocations []api.ResolvedLocation) ([]APILocation, error) { + var apiLocations []APILocation + for _, res := range resolvedLocations { + apiLocations = append(apiLocations, APILocation{ + RepositoryID: res.Dump.RepositoryID, + Commit: res.Dump.Commit, + Path: res.Path, + Range: res.Range, + }) + } + + return apiLocations, nil +} diff --git a/internal/codeintel/lsifserver/client/proxy.go b/internal/codeintel/lsifserver/client/proxy.go index 109a101c96e2a2..51d4c64c796efe 100644 --- a/internal/codeintel/lsifserver/client/proxy.go +++ b/internal/codeintel/lsifserver/client/proxy.go @@ -1,10 +1,15 @@ package client import ( + "bytes" "context" "fmt" + "io" + "io/ioutil" "net/http" + "net/http/httptest" + "github.com/gorilla/mux" "github.com/opentracing-contrib/go-stdlib/nethttp" "github.com/opentracing/opentracing-go/ext" "github.com/pkg/errors" @@ -12,6 +17,36 @@ import ( ) func (c *Client) RawRequest(ctx context.Context, req *http.Request) (_ *http.Response, err error) { + router := mux.NewRouter() + router.Path("/uploads/{id:[0-9]+}").Methods("GET").HandlerFunc(c.server.handleGetUploadByID) + router.Path("/uploads/{id:[0-9]+}").Methods("DELETE").HandlerFunc(c.server.handleDeleteUploadByID) + router.Path("/uploads/repository/{id:[0-9]+}").Methods("GET").HandlerFunc(c.server.handleGetUploadsByRepo) + router.Path("/exists").Methods("GET").HandlerFunc(c.server.handleExists) + router.Path("/definitions").Methods("GET").HandlerFunc(c.server.handleDefinitions) + router.Path("/references").Methods("GET").HandlerFunc(c.server.handleReferences) + router.Path("/hover").Methods("GET").HandlerFunc(c.server.handleHover) + router.Path("/upload").Methods("POST").HandlerFunc(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + resp, err := c.rawRequest(ctx, req) + if err != nil { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(err.Error())) + return + } + + w.WriteHeader(resp.StatusCode) + _, _ = io.Copy(w, resp.Body) + })) + + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return &http.Response{ + StatusCode: rec.Code, + Header: rec.Header(), + Body: ioutil.NopCloser(bytes.NewReader(rec.Body.Bytes())), + }, nil +} + +func (c *Client) rawRequest(ctx context.Context, req *http.Request) (_ *http.Response, err error) { span, ctx := ot.StartSpanFromContext(ctx, "lsifserver.client.do") defer func() { if err != nil { diff --git a/internal/codeintel/lsifserver/client/server.go b/internal/codeintel/lsifserver/client/server.go new file mode 100644 index 00000000000000..a0ba3b69ac5149 --- /dev/null +++ b/internal/codeintel/lsifserver/client/server.go @@ -0,0 +1,25 @@ +package client + +import ( + "github.com/sourcegraph/sourcegraph/internal/codeintel/api" + bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" + "github.com/sourcegraph/sourcegraph/internal/codeintel/db" +) + +type Server struct { + db db.DB + bundleManagerClient bundles.BundleManagerClient + codeIntelAPI api.CodeIntelAPI +} + +func NewServer( + db db.DB, + bundleManagerClient bundles.BundleManagerClient, + codeIntelAPI api.CodeIntelAPI, +) *Server { + return &Server{ + db: db, + bundleManagerClient: bundleManagerClient, + codeIntelAPI: codeIntelAPI, + } +} diff --git a/internal/codeintel/lsifserver/client/util.go b/internal/codeintel/lsifserver/client/util.go new file mode 100644 index 00000000000000..d92e88b078b642 --- /dev/null +++ b/internal/codeintel/lsifserver/client/util.go @@ -0,0 +1,94 @@ +package client + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "strings" + + "github.com/gorilla/mux" + "github.com/inconshreveable/log15" + "github.com/tomnomnom/linkheader" +) + +func hasQuery(r *http.Request, name string) bool { + return r.URL.Query().Get(name) != "" +} + +func getQuery(r *http.Request, name string) string { + return r.URL.Query().Get(name) +} + +func getQueryInt(r *http.Request, name string) int { + value, _ := strconv.Atoi(r.URL.Query().Get(name)) + return value +} + +func getQueryIntDefault(r *http.Request, name string, defaultValue int) int { + value, err := strconv.Atoi(r.URL.Query().Get(name)) + if err != nil { + value = defaultValue + } + return value +} + +func getQueryBool(r *http.Request, name string) bool { + value, _ := strconv.ParseBool(r.URL.Query().Get(name)) + return value +} + +func makeNextLink(url *url.URL, newQueryValues map[string]interface{}) string { + q := url.Query() + for k, v := range newQueryValues { + q.Set(k, fmt.Sprintf("%v", v)) + } + url.RawQuery = q.Encode() + + header := linkheader.Link{ + URL: url.String(), + Rel: "next", + } + return header.String() +} + +// idFromRequest returns the database id from the request URL's path. This method +// must only be called from routes containing the `id:[0-9]+` pattern, as the error +// return from ParseInt is not checked. +func idFromRequest(r *http.Request) int64 { + id, _ := strconv.ParseInt(mux.Vars(r)["id"], 10, 64) + return id +} + +// copyAll writes the contents of r to w and logs on write failure. +func copyAll(w http.ResponseWriter, r io.Reader) { + if _, err := io.Copy(w, r); err != nil { + log15.Error("Failed to write payload to client", "error", err) + } +} + +// writeJSON writes the JSON-encoded payload to w and logs on write failure. +// If there is an encoding error, then a 500-level status is written to w. +func writeJSON(w http.ResponseWriter, payload interface{}) { + data, err := json.Marshal(payload) + if err != nil { + log15.Error("Failed to serialize result", "error", err) + http.Error(w, fmt.Sprintf("failed to serialize result: %s", err.Error()), http.StatusInternalServerError) + return + } + + copyAll(w, bytes.NewReader(data)) +} + +func sanitizeRoot(s string) string { + if s == "" || s == "/" { + return "" + } + if !strings.HasSuffix(s, "/") { + s += "/" + } + return s +} From 871cf0004040e9091c7233dba69b52bf28cfa469 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Wed, 20 May 2020 11:58:53 -0500 Subject: [PATCH 09/11] Fix tests. --- internal/codeintel/api/helpers_test.go | 2 +- .../codeintel/api}/testdata/filters/normal/1 | 0 .../codeintel/api}/testdata/filters/normal/10 | 0 .../codeintel/api}/testdata/filters/normal/11 | 0 .../codeintel/api}/testdata/filters/normal/12 | 0 .../codeintel/api}/testdata/filters/normal/2 | 0 .../codeintel/api}/testdata/filters/normal/3 | 0 .../codeintel/api}/testdata/filters/normal/4 | 0 .../codeintel/api}/testdata/filters/normal/5 | 0 .../codeintel/api}/testdata/filters/normal/6 | 0 .../codeintel/api}/testdata/filters/normal/7 | 0 .../codeintel/api}/testdata/filters/normal/8 | 0 .../codeintel/api}/testdata/filters/normal/9 | 0 13 files changed, 1 insertion(+), 1 deletion(-) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/1 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/10 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/11 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/12 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/2 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/3 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/4 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/5 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/6 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/7 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/8 (100%) rename {cmd/precise-code-intel-api-server => internal/codeintel/api}/testdata/filters/normal/9 (100%) diff --git a/internal/codeintel/api/helpers_test.go b/internal/codeintel/api/helpers_test.go index ffa9f779c7c6ae..121578d1169d97 100644 --- a/internal/codeintel/api/helpers_test.go +++ b/internal/codeintel/api/helpers_test.go @@ -271,7 +271,7 @@ func setMockBundleClientPackageInformation(t *testing.T, mockBundleClient *bundl } func readTestFilter(t *testing.T, dirname, filename string) []byte { - content, err := ioutil.ReadFile(fmt.Sprintf("../../testdata/filters/%s/%s", dirname, filename)) + content, err := ioutil.ReadFile(fmt.Sprintf("./testdata/filters/%s/%s", dirname, filename)) if err != nil { t.Fatalf("unexpected error reading: %s", err) } diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/1 b/internal/codeintel/api/testdata/filters/normal/1 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/1 rename to internal/codeintel/api/testdata/filters/normal/1 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/10 b/internal/codeintel/api/testdata/filters/normal/10 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/10 rename to internal/codeintel/api/testdata/filters/normal/10 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/11 b/internal/codeintel/api/testdata/filters/normal/11 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/11 rename to internal/codeintel/api/testdata/filters/normal/11 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/12 b/internal/codeintel/api/testdata/filters/normal/12 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/12 rename to internal/codeintel/api/testdata/filters/normal/12 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/2 b/internal/codeintel/api/testdata/filters/normal/2 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/2 rename to internal/codeintel/api/testdata/filters/normal/2 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/3 b/internal/codeintel/api/testdata/filters/normal/3 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/3 rename to internal/codeintel/api/testdata/filters/normal/3 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/4 b/internal/codeintel/api/testdata/filters/normal/4 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/4 rename to internal/codeintel/api/testdata/filters/normal/4 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/5 b/internal/codeintel/api/testdata/filters/normal/5 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/5 rename to internal/codeintel/api/testdata/filters/normal/5 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/6 b/internal/codeintel/api/testdata/filters/normal/6 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/6 rename to internal/codeintel/api/testdata/filters/normal/6 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/7 b/internal/codeintel/api/testdata/filters/normal/7 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/7 rename to internal/codeintel/api/testdata/filters/normal/7 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/8 b/internal/codeintel/api/testdata/filters/normal/8 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/8 rename to internal/codeintel/api/testdata/filters/normal/8 diff --git a/cmd/precise-code-intel-api-server/testdata/filters/normal/9 b/internal/codeintel/api/testdata/filters/normal/9 similarity index 100% rename from cmd/precise-code-intel-api-server/testdata/filters/normal/9 rename to internal/codeintel/api/testdata/filters/normal/9 From 0a4c63043cc61b1dfb01a8d960b2273b74e31612 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Wed, 20 May 2020 15:34:38 -0500 Subject: [PATCH 10/11] Fix build. --- .../internal/paths/exists.go | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 cmd/precise-code-intel-bundle-manager/internal/paths/exists.go diff --git a/cmd/precise-code-intel-bundle-manager/internal/paths/exists.go b/cmd/precise-code-intel-bundle-manager/internal/paths/exists.go deleted file mode 100644 index 2b93b9d9799e26..00000000000000 --- a/cmd/precise-code-intel-bundle-manager/internal/paths/exists.go +++ /dev/null @@ -1,15 +0,0 @@ -package paths - -import "os" - -func PathExists(filename string) (bool, error) { - if _, err := os.Stat(filename); err != nil { - if os.IsNotExist(err) { - return false, nil - } - - return false, err - } - - return true, nil -} From bc072662500da3ce0bc7b5820bf0f63fb59182fb Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 21 May 2020 09:37:43 -0500 Subject: [PATCH 11/11] Reduce setup code. --- enterprise/cmd/frontend/main.go | 18 ++++-------------- .../codeintel/lsifserver/proxy/proxy.go | 6 ++---- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/enterprise/cmd/frontend/main.go b/enterprise/cmd/frontend/main.go index 53b54d7917b1d5..e6e564c417ba22 100644 --- a/enterprise/cmd/frontend/main.go +++ b/enterprise/cmd/frontend/main.go @@ -35,6 +35,7 @@ import ( codeintelapi "github.com/sourcegraph/sourcegraph/internal/codeintel/api" bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" codeinteldb "github.com/sourcegraph/sourcegraph/internal/codeintel/db" + "github.com/sourcegraph/sourcegraph/internal/codeintel/enqueuer" codeintelgitserver "github.com/sourcegraph/sourcegraph/internal/codeintel/gitserver" lsifserverclient "github.com/sourcegraph/sourcegraph/internal/codeintel/lsifserver/client" "github.com/sourcegraph/sourcegraph/internal/conf" @@ -183,25 +184,14 @@ func initCodeIntel() { codeintelapi.New(db, bundleManagerClient, codeintelgitserver.DefaultClient), ) + enqueuer := enqueuer.NewEnqueuer(db, bundleManagerClient) + graphqlbackend.NewCodeIntelResolver = func() graphqlbackend.CodeIntelResolver { return codeIntelResolvers.NewResolver(client) } httpapi.NewLSIFServerProxy = func() (*httpapi.LSIFServerProxy, error) { - if bundleManagerURL == "" { - log.Fatalf("invalid value for PRECISE_CODE_INTEL_BUNDLE_MANAGER_URL: no value supplied") - } - - observationContext := &observation.Context{ - Logger: log15.Root(), - Tracer: &trace.Tracer{Tracer: opentracing.GlobalTracer()}, - Registerer: prometheus.DefaultRegisterer, - } - - db := codeinteldb.NewObserved(codeinteldb.NewWithHandle(dbconn.Global), observationContext) - bundleManagerClient := bundles.New(bundleManagerURL) - - return proxy.NewProxy(db, bundleManagerClient) + return proxy.NewProxy(enqueuer, client) } } diff --git a/enterprise/internal/codeintel/lsifserver/proxy/proxy.go b/enterprise/internal/codeintel/lsifserver/proxy/proxy.go index 0950ee983c5d0b..a88f8e96cc97a0 100644 --- a/enterprise/internal/codeintel/lsifserver/proxy/proxy.go +++ b/enterprise/internal/codeintel/lsifserver/proxy/proxy.go @@ -14,8 +14,6 @@ import ( "github.com/sourcegraph/sourcegraph/cmd/frontend/httpapi" "github.com/sourcegraph/sourcegraph/cmd/frontend/types" "github.com/sourcegraph/sourcegraph/internal/api" - bundles "github.com/sourcegraph/sourcegraph/internal/codeintel/bundles/client" - codeinteldb "github.com/sourcegraph/sourcegraph/internal/codeintel/db" "github.com/sourcegraph/sourcegraph/internal/codeintel/enqueuer" "github.com/sourcegraph/sourcegraph/internal/codeintel/lsifserver/client" "github.com/sourcegraph/sourcegraph/internal/conf" @@ -23,9 +21,9 @@ import ( "github.com/sourcegraph/sourcegraph/internal/gitserver" ) -func NewProxy(db codeinteldb.DB, bundleManagerClient bundles.BundleManagerClient, lsifserverClient *client.Client) (*httpapi.LSIFServerProxy, error) { +func NewProxy(enqueuer *enqueuer.Enqueuer, lsifserverClient *client.Client) (*httpapi.LSIFServerProxy, error) { return &httpapi.LSIFServerProxy{ - UploadHandler: http.HandlerFunc(uploadProxyHandler(enqueuer.NewEnqueuer(db, bundleManagerClient), lsifserverClient)), + UploadHandler: http.HandlerFunc(uploadProxyHandler(enqueuer, lsifserverClient)), }, nil }