diff options
| author | Paul Okstad <pokstad@gitlab.com> | 2020-11-19 20:54:54 +0300 |
|---|---|---|
| committer | Paul Okstad <pokstad@gitlab.com> | 2020-11-19 20:54:54 +0300 |
| commit | d580e7718921a6d39e77bcdf1abb12cfa178ab7c (patch) | |
| tree | 059e276649284f7b21cfed2260eb14c6c901dd4c | |
| parent | cf64ad3cf427a0a57196abf94ece23e7619a9253 (diff) | |
| parent | 8b9d8f482ae61fcc5ca973f4e5552e4c702519d1 (diff) | |
Merge branch 'pks-log-command-stats-fix-segfault' into 'master'
command: Fix segfault with command stats
Closes #3280
See merge request gitlab-org/gitaly!2791
| -rw-r--r-- | changelogs/unreleased/pks-log-command-stats-fix-segfault.yml | 5 | ||||
| -rw-r--r-- | internal/command/command.go | 4 | ||||
| -rw-r--r-- | internal/gitaly/service/smarthttp/upload_pack.go | 3 | ||||
| -rw-r--r-- | internal/gitaly/service/ssh/upload_pack.go | 3 | ||||
| -rw-r--r-- | internal/metadata/featureflag/feature_flags.go | 1 |
5 files changed, 13 insertions, 3 deletions
diff --git a/changelogs/unreleased/pks-log-command-stats-fix-segfault.yml b/changelogs/unreleased/pks-log-command-stats-fix-segfault.yml new file mode 100644 index 000000000..de151c683 --- /dev/null +++ b/changelogs/unreleased/pks-log-command-stats-fix-segfault.yml @@ -0,0 +1,5 @@ +--- +title: 'command: Fix panics and segfaults caused by LogCommandStats' +merge_request: 2791 +author: +type: fixed diff --git a/internal/command/command.go b/internal/command/command.go index de3a91f3c..59d06a287 100644 --- a/internal/command/command.go +++ b/internal/command/command.go @@ -431,9 +431,7 @@ func (c *Command) logProcessComplete(ctx context.Context, exitCode int) { entry.Debug("spawn complete") - if featureflag.IsEnabled(ctx, featureflag.LogCommandStats) { - stats := StatsFromContext(ctx) - + if stats := StatsFromContext(ctx); stats != nil && featureflag.IsEnabled(ctx, featureflag.LogCommandStats) { stats.RecordSum("command.count", 1) stats.RecordSum("command.system_time_ms", int(systemTime.Seconds()*1000)) stats.RecordSum("command.user_time_ms", int(userTime.Seconds()*1000)) diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go index ccab8d2b7..f501ad397 100644 --- a/internal/gitaly/service/smarthttp/upload_pack.go +++ b/internal/gitaly/service/smarthttp/upload_pack.go @@ -101,6 +101,9 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS return status.Errorf(codes.Unavailable, "PostUploadPack: %v", err) } + pw.Close() // Ensure PackfileNegotiation parser returns + <-statsCh // Wait for the packfile negotiation parser to finish. + ctxlogrus.Extract(ctx).WithField("request_sha", fmt.Sprintf("%x", h.Sum(nil))).WithField("response_bytes", respBytes).Info("request details") return nil diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go index f85b27751..8bb1aed2d 100644 --- a/internal/gitaly/service/ssh/upload_pack.go +++ b/internal/gitaly/service/ssh/upload_pack.go @@ -121,6 +121,9 @@ func (s *server) sshUploadPack(stream gitalypb.SSHService_SSHUploadPackServer, r go monitor.Monitor(pktline.PktDone(), s.uploadPackRequestTimeout, cancelCtx) if err := cmd.Wait(); err != nil { + pw.Close() + wg.Wait() + if status, ok := command.ExitStatus(err); ok { return stream.Send(&gitalypb.SSHUploadPackResponse{ ExitStatus: &gitalypb.ExitStatus{Value: int32(status)}, diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go index 3878bc805..9a0f48187 100644 --- a/internal/metadata/featureflag/feature_flags.go +++ b/internal/metadata/featureflag/feature_flags.go @@ -48,6 +48,7 @@ var All = []FeatureFlag{ GoFetchSourceBranch, DistributedReads, RubyReferenceTransactionHook, + LogCommandStats, GoUserMergeBranch, GoUserMergeToRef, GoUserFFBranch, |