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

Remove handling of unsupported --mem-ballast-size-mib command line argument #2339

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

dmitryax
Copy link
Contributor

@dmitryax dmitryax commented Dec 9, 2022

The argument was removed from the core collector in v0.36.0. We kept supporting it in our distro but the support is broken. We never remove that argument as we do for others and it's being passed to the core collector in https://github.com/signalfx/splunk-otel-collector/blob/main/cmd/otelcol/main.go#L105 where it fails with the following error:

2022/12/08 16:25:34 main.go:106: application run finished with error: unknown flag: --mem-ballast-size-mib

So I don't think it makes sense to bring back support of it.

@dmitryax dmitryax requested review from a team as code owners December 9, 2022 00:51
@dmitryax dmitryax changed the title Remove --mem-ballast-size-mib handling of arg Remove handling of broken --mem-ballast-size-mib command line argument Dec 9, 2022
@dmitryax dmitryax changed the title Remove handling of broken --mem-ballast-size-mib command line argument Remove handling of unsupported --mem-ballast-size-mib command line argument Dec 9, 2022
@rmfitzpatrick
Copy link
Contributor

Considering this hasn't worked for a bit it makes sense to remove as you have, but it could also be restored but no longer passed to the service w/ removeFlag(), which would be a simpler change (fwiw).

@dmitryax
Copy link
Contributor Author

dmitryax commented Dec 9, 2022

Considering this hasn't worked for a bit it makes sense to remove it as you have, but it could also be restored but no longer passed to the service w/ removeFlag(), which would be a simpler change (fwiw).

Yes, I understand that, but given that it has been broken for a while, I think it's safe to remove which has to be done anyway at some point I believe

The argument was removed from the core collector in v0.36.0. We kept supporting it in our distro but the support is broken. We never remove that argument as we do for others and it's being passed to the core collector in https://github.com/signalfx/splunk-otel-collector/blob/main/cmd/otelcol/main.go#L105 where it fails with the following error:

```
2022/12/08 16:25:34 main.go:106: application run finished with error: unknown flag: --mem-ballast-size-mib
```

So I don't think it makes sense to bring back the support of it
@dmitryax dmitryax force-pushed the remove-mem-ballast-size-mib branch from e69b188 to 8a22918 Compare December 9, 2022 01:28
@dmitryax
Copy link
Contributor Author

dmitryax commented Dec 9, 2022

I don't think there is a reason to keep and maintain an additional configuration option which is likely not being used given that there were no complaints since 0.36.0

@dmitryax dmitryax merged commit 3e74559 into main Dec 9, 2022
@delete-merged-branch delete-merged-branch bot deleted the remove-mem-ballast-size-mib branch December 9, 2022 01:58
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.

3 participants