Skip to content

chore: Follow up to #731 #743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
efritz merged 5 commits into from
May 18, 2022
Merged

chore: Follow up to #731 #743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
461ad1c
Make boolean flag consistent.
efritz May 13, 2022
656391e
Regroup imports.
efritz May 13, 2022
1b0836b
Fix docstring.
efritz May 13, 2022
df58c67
Update size of semaphores.
efritz May 13, 2022
83109fe
Add newline.
efritz May 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions cmd/src/debug_compose.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"

"github.com/sourcegraph/sourcegraph/lib/errors"

"golang.org/x/sync/errgroup"

"golang.org/x/sync/semaphore"
)

Expand Down Expand Up @@ -42,9 +41,9 @@ Examples:

flagSet := flag.NewFlagSet("compose", flag.ExitOnError)
var base string
var noConfigs bool
var excludeConfigs bool
flagSet.StringVar(&base, "o", "debug.zip", "The name of the output zip archive")
flagSet.BoolVar(&noConfigs, "no-configs", false, "If true include Sourcegraph configuration files. Default value true.")
flagSet.BoolVar(&excludeConfigs, "no-configs", false, "If true, exclude Sourcegraph configuration files. Defaults to false.")

handler := func(args []string) error {
if err := flagSet.Parse(args); err != nil {
Expand Down Expand Up @@ -81,7 +80,7 @@ Examples:
return nil
}

err = archiveCompose(ctx, zw, *verbose, noConfigs, baseDir)
err = archiveCompose(ctx, zw, *verbose, !excludeConfigs, baseDir)
if err != nil {
return err
}
Expand All @@ -98,7 +97,7 @@ Examples:
}

// writes archive of common docker cli commands
func archiveCompose(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool, baseDir string) error {
func archiveCompose(ctx context.Context, zw *zip.Writer, verbose, archiveConfigs bool, baseDir string) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

Expand All @@ -114,7 +113,7 @@ func archiveCompose(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool
// setup channel for slice of archive function outputs
ch := make(chan *archiveFile)
g, ctx := errgroup.WithContext(ctx)
semaphore := semaphore.NewWeighted(8)
semaphore := semaphore.NewWeighted(int64(runtime.GOMAXPROCS(0)))
Copy link
Contributor

@DaedalusG DaedalusG May 17, 2022

Choose a reason for hiding this comment

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

This one is interesting, I'm not familiar with runtime.GOMAXPROCS(0), will definitely do some research here 👍

Copy link
Contributor Author

@efritz efritz May 17, 2022

Choose a reason for hiding this comment

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

It gives you the recommended number of processes/goroutines that can run in parallel (not just concurrently). It's relational to your cpu die count.

Copy link
Contributor

@DaedalusG DaedalusG May 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, will this potentially reintroduce issues with open file limits? Or reduce the effectiveness of the semaphore in throttling remotely executed requests? In my understanding, this will scale the semaphore's limit to the host machine running src's resources?

Copy link
Contributor Author

@efritz efritz May 18, 2022

Choose a reason for hiding this comment

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

Number of cpu cores is way under the Linux file handle limit by many orders of magnitude


run := func(f func() *archiveFile) {
g.Go(func() error {
Expand Down Expand Up @@ -150,7 +149,7 @@ func archiveCompose(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool
}

// start goroutine to get configs
if !noConfigs {
if archiveConfigs {
run(func() *archiveFile { return getExternalServicesConfig(ctx, baseDir) })

run(func() *archiveFile { return getSiteConfig(ctx, baseDir) })
Expand Down
17 changes: 9 additions & 8 deletions cmd/src/debug_kube.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,19 @@ import (
"log"
"os"
"path/filepath"
"runtime"

"github.com/sourcegraph/sourcegraph/lib/errors"
"golang.org/x/sync/errgroup"

"golang.org/x/sync/semaphore"

"github.com/sourcegraph/sourcegraph/lib/errors"
"github.com/sourcegraph/src-cli/internal/exec"
)

func init() {
usage := `
'src debug kube' invokes kubectl diagnostic commands targeting kubectl's current-context, writing returns to an archive.

Usage:

src debug kube [command options]
Expand All @@ -45,10 +46,10 @@ Examples:
flagSet := flag.NewFlagSet("kube", flag.ExitOnError)
var base string
var namespace string
var noConfigs bool
var excludeConfigs bool
flagSet.StringVar(&base, "o", "debug.zip", "The name of the output zip archive")
flagSet.StringVar(&namespace, "n", "default", "The namespace passed to kubectl commands, if not specified the 'default' namespace is used")
flagSet.BoolVar(&noConfigs, "no-configs", false, "If true include Sourcegraph configuration files. Default value true.")
flagSet.BoolVar(&excludeConfigs, "no-configs", false, "If true, exclude Sourcegraph configuration files. Defaults to false.")

handler := func(args []string) error {
if err := flagSet.Parse(args); err != nil {
Expand Down Expand Up @@ -88,7 +89,7 @@ Examples:
return nil
}

err = archiveKube(ctx, zw, *verbose, noConfigs, namespace, baseDir, pods)
err = archiveKube(ctx, zw, *verbose, !excludeConfigs, namespace, baseDir, pods)
if err != nil {
return err
}
Expand Down Expand Up @@ -118,7 +119,7 @@ type podList struct {
}

// Runs common kubectl functions and archive results to zip file
func archiveKube(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool, namespace, baseDir string, pods podList) error {
func archiveKube(ctx context.Context, zw *zip.Writer, verbose, archiveConfigs bool, namespace, baseDir string, pods podList) error {
// Create a context with a cancel function that we call when returning
// from archiveKube. This ensures we close all pending go-routines when returning
// early because of an error.
Expand All @@ -128,7 +129,7 @@ func archiveKube(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool, n
// setup channel for slice of archive function outputs, as well as throttling semaphore
ch := make(chan *archiveFile)
g, ctx := errgroup.WithContext(ctx)
semaphore := semaphore.NewWeighted(8)
semaphore := semaphore.NewWeighted(int64(runtime.GOMAXPROCS(0)))

run := func(f func() *archiveFile) {
g.Go(func() error {
Expand Down Expand Up @@ -198,7 +199,7 @@ func archiveKube(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool, n
}

// start goroutine to get external service config
if !noConfigs {
if archiveConfigs {
run(func() *archiveFile { return getExternalServicesConfig(ctx, baseDir) })

run(func() *archiveFile { return getSiteConfig(ctx, baseDir) })
Expand Down
16 changes: 8 additions & 8 deletions cmd/src/debug_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import (
"log"
"os"
"path/filepath"
"runtime"

"github.com/sourcegraph/sourcegraph/lib/errors"
"golang.org/x/sync/errgroup"
"golang.org/x/sync/semaphore"

"github.com/sourcegraph/sourcegraph/lib/errors"
)

func init() {
Expand Down Expand Up @@ -40,10 +40,10 @@ Examples:
flagSet := flag.NewFlagSet("server", flag.ExitOnError)
var base string
var container string
var noConfigs bool
var excludeConfigs bool
flagSet.StringVar(&base, "o", "debug.zip", "The name of the output zip archive")
flagSet.StringVar(&container, "c", "", "The container to target")
flagSet.BoolVar(&noConfigs, "no-configs", false, "If true include Sourcegraph configuration files. Default value true.")
flagSet.BoolVar(&excludeConfigs, "no-configs", false, "If true, exclude Sourcegraph configuration files. Defaults to false.")

handler := func(args []string) error {
if err := flagSet.Parse(args); err != nil {
Expand Down Expand Up @@ -77,7 +77,7 @@ Examples:
return nil
}

err = archiveServ(ctx, zw, *verbose, noConfigs, container, baseDir)
err = archiveServ(ctx, zw, *verbose, !excludeConfigs, container, baseDir)
if err != nil {
return err
}
Expand All @@ -94,14 +94,14 @@ Examples:
}

// Runs common docker cli commands on a single container
func archiveServ(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool, container, baseDir string) error {
func archiveServ(ctx context.Context, zw *zip.Writer, verbose, archiveConfigs bool, container, baseDir string) error {
ctx, cancel := context.WithCancel(ctx)
defer cancel()

// setup channel for slice of archive function outputs
ch := make(chan *archiveFile)
g, ctx := errgroup.WithContext(ctx)
semaphore := semaphore.NewWeighted(8)
semaphore := semaphore.NewWeighted(int64(runtime.GOMAXPROCS(0)))

run := func(f func() *archiveFile) {
g.Go(func() error {
Expand All @@ -128,7 +128,7 @@ func archiveServ(ctx context.Context, zw *zip.Writer, verbose, noConfigs bool, c
run(func() *archiveFile { return getServTop(ctx, container, baseDir) })

// start goroutine to get configs
if !noConfigs {
if archiveConfigs {
run(func() *archiveFile { return getExternalServicesConfig(ctx, baseDir) })

run(func() *archiveFile { return getSiteConfig(ctx, baseDir) })
Expand Down