Skip to content

Commit

Permalink
fix: Fixing concurrent access to read files map (#3615)
Browse files Browse the repository at this point in the history
* fix: Fixing concurrent access to read files map

* fix: Strangely it's still not working. Getting help.

* fix: Removing debug code

* fix: Making the insert atomic

* chore: Adding units reading race condition test

* feat: Adding test to hopefully trigger race condition in concurrent units reading test

* fix: These are called units

* fix: I guess I have to use `CopyFolderContents`

* fix: Removing unused import

* fix: Fixing ephemeral unit generation

* fix: Use unit names instead of directories

* feat: Increasing load to increase likliehood of race condition

* Revert "Merge branch 'chore/adding-units-reading-race-condition-test' into yousif/tg-655-fatal-error-concurrent-map-writes-gitlab-pipeline-when"

This reverts commit 0518d96, reversing
changes made to f70dc72.

* fix: Adding test specifically for the race condition

* feat: Adding extra test for race conditions

* fix: Adjusting jobs so that race tests run as their own jobs

* fix: Forgot to also add them here

* fix: I immediately knew I'd regret that
  • Loading branch information
yhakbar authored Dec 3, 2024
1 parent 43d03d3 commit 0e8342f
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 9 deletions.
32 changes: 32 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ run_tflint_test: &run_tflint_test
run-go-tests --packages "-tags tflint -run TestTflint ./test" | tee logs/test-results.log
no_output_timeout: 30m

run_race_test: &run_race_test
name: Run Race tests
command: |
run-go-tests --packages "-run '.*WithRacing' -race ./test" | tee logs/test-results.log
no_output_timeout: 30m

run_terratest_log_parser: &run_terratest_log_parser
name: Terratest log parser
command: |
Expand Down Expand Up @@ -519,6 +525,24 @@ jobs:
- store_test_results:
path: logs

race_test:
resource_class: medium
<<: *defaults
steps:
- checkout
- run:
<<: *install_tofu
- run:
<<: *setup_test_environment
- run:
<<: *run_race_test
- run:
<<: *run_terratest_log_parser
- store_artifacts:
path: logs
- store_test_results:
path: logs

run_markdownlint:
<<: *defaults
steps:
Expand Down Expand Up @@ -719,6 +743,14 @@ workflows:
- AWS__PHXDEVOPS__circle-ci-test
- GCP__automated-tests
- GITHUB__PAT__gruntwork-ci
- race_test:
filters:
tags:
only: /^v.*/
context:
- AWS__PHXDEVOPS__circle-ci-test
- GCP__automated-tests
- GITHUB__PAT__gruntwork-ci
- integration_test_tofu_engine:
filters:
tags:
Expand Down
47 changes: 38 additions & 9 deletions options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/gruntwork-io/terragrunt/pkg/log/format/placeholders"
"github.com/gruntwork-io/terragrunt/util"
"github.com/hashicorp/go-version"
"github.com/puzpuzpuz/xsync/v3"
)

const ContextKey ctxKey = iota
Expand Down Expand Up @@ -364,7 +365,7 @@ type TerragruntOptions struct {

// ReadFiles is a map of files to the Units
// that read them using HCL functions in the unit.
ReadFiles map[string][]string
ReadFiles *xsync.MapOf[string, []string]
}

// TerragruntOptionsFunc is a functional option type used to pass options in certain integration tests
Expand Down Expand Up @@ -477,6 +478,7 @@ func NewTerragruntOptionsWithWriters(stdout, stderr io.Writer) *TerragruntOption
OutputFolder: "",
JSONOutputFolder: "",
FeatureFlags: map[string]string{},
ReadFiles: xsync.NewMapOf[string, []string](),
}
}

Expand Down Expand Up @@ -737,17 +739,37 @@ func (opts *TerragruntOptions) DataDir() string {
// AppendReadFile appends to the list of files read by a given unit.
func (opts *TerragruntOptions) AppendReadFile(file, unit string) {
if opts.ReadFiles == nil {
opts.ReadFiles = map[string][]string{}
opts.ReadFiles = xsync.NewMapOf[string, []string]()
}

for _, u := range opts.ReadFiles[file] {
units, ok := opts.ReadFiles.Load(file)
if !ok {
opts.ReadFiles.Store(file, []string{unit})
return
}

for _, u := range units {
if u == unit {
return
}
}

opts.Logger.Debugf("Tracking that file %s was read by %s.", file, unit)
opts.ReadFiles[file] = append(opts.ReadFiles[file], unit)

// Atomic insert
// https://github.com/puzpuzpuz/xsync/issues/123#issuecomment-1963458519
_, _ = opts.ReadFiles.Compute(file, func(oldUnits []string, loaded bool) ([]string, bool) {
var newUnits []string

if loaded {
newUnits = append(make([]string, 0, len(oldUnits)+1), oldUnits...)
newUnits = append(newUnits, unit)
} else {
newUnits = []string{unit}
}

return newUnits, false
})
}

// DidReadFile checks if a given file was read by a given unit.
Expand All @@ -756,7 +778,12 @@ func (opts *TerragruntOptions) DidReadFile(file, unit string) bool {
return false
}

for _, u := range opts.ReadFiles[file] {
units, ok := opts.ReadFiles.Load(file)
if !ok {
return false
}

for _, u := range units {
if u == unit {
return true
}
Expand All @@ -766,16 +793,18 @@ func (opts *TerragruntOptions) DidReadFile(file, unit string) bool {
}

// CloneReadFiles creates a copy of the ReadFiles map.
func (opts *TerragruntOptions) CloneReadFiles(readFiles map[string][]string) {
func (opts *TerragruntOptions) CloneReadFiles(readFiles *xsync.MapOf[string, []string]) {
if readFiles == nil {
return
}

for file, units := range readFiles {
readFiles.Range(func(key string, units []string) bool {
for _, unit := range units {
opts.AppendReadFile(file, unit)
opts.AppendReadFile(key, unit)
}
}

return true
})
}

// identifyDefaultWrappedExecutable returns default path used for wrapped executable.
Expand Down
20 changes: 20 additions & 0 deletions test/race_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Tests specific to race conditions are verified here

package test_test

import (
"testing"

"github.com/gruntwork-io/terragrunt/options"
"github.com/stretchr/testify/require"
)

func TestStrictModeWithRacing(t *testing.T) {
t.Parallel()

opts, err := options.NewTerragruntOptionsForTest("race_test")
require.NoError(t, err)

go opts.AppendReadFile("file.json", "unit")
go opts.AppendReadFile("file.json", "other-unit")
}

0 comments on commit 0e8342f

Please sign in to comment.