Skip to content
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.

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

fix: only add arn patterns to list if not already contained #47

Merged
merged 1 commit into from
Mar 7, 2022
Merged

fix: only add arn patterns to list if not already contained #47

merged 1 commit into from
Mar 7, 2022

Conversation

cfstras
Copy link
Contributor

@cfstras cfstras commented Mar 7, 2022

I found a case where dynamoDB requests would create infinite (or just very very large) lists. This led to iamlive pinning one CPU at 100%, and not making progress on the request being processed.

See below an output from my dlv session:

> github.com/iann0036/iamlive/iamlivecore.subARNParameters() ./iamlivecore/logger.go:658 (PC: 0x100bdec38)
   653:		// parameter substitution
   654:		for paramVarName, params := range call.Parameters {
   655:			newArns := []string{}
   656:			for _, param := range params {
   657:				for _, arn := range arns {
=> 658:					newArns = append(newArns, regexp.MustCompile(`\$\{`+strings.ReplaceAll(strings.ReplaceAll(paramVarName, "[", "\\["), "]", "\\]")+`\}`).ReplaceAllString(arn, param)) // might have dupes but resolved out later
   659:				}
   660:			}
   661:			arns = newArns
   662:		}
   663:
(dlv) p arns
[]string len: 524288, cap: 568320, [
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",
	"arn:${Partition}:dynamodb:${Region}:${Account}:table/${TableName...+1 more",

I slightly refactored this section in the code, and added a set along with the list. If the ordering is not important here, we could also remove the list entirely, or simplify the code further (I didn't look too hard, just removed the dupes situation for now.

With this fix, my requests run fine.

@iann0036
Copy link
Owner

iann0036 commented Mar 7, 2022

Hey @cfstras,

Thanks for this contribution and your brief testing!

I'll include this in the next release today or tomorrow.

@iann0036 iann0036 merged commit 88b1c0a into iann0036:main Mar 7, 2022
@cfstras cfstras deleted the fix-exponential-regexps branch March 8, 2022 08:38
@cfstras
Copy link
Contributor Author

cfstras commented Mar 8, 2022

Glad to have helped, thanks for merging 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants