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 for concurrent modification exception on removing table #78

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

podile
Copy link

@podile podile commented Mar 15, 2020

Fix for the issue #77

I am able reproduce the issue with below code snippet.

`package com.zegelin.cassandra.exporter;

import com.google.common.collect.Sets;
import java.util.Set;

public class TestSets {

public static void main(String args[]) {
    Set<String> set1 = Sets.newHashSet("E1", "E2", "E3", "E4", "E5", "E6");
    Set<String> set2 = Sets.newHashSet("E4", "E5", "E6");
    Set<String> removedMBeans = Sets.difference(set1, set2);
    for (String e : removedMBeans) {
        set1.remove(e);
    }
}

}
`

zegelin and others added 15 commits February 20, 2020 18:12
Enhanced code to filter quantiles based on user choice
Included --cql-ssl option to crated encrypted native connection
Added latest version of jars as the older version are having some
vulnerabilities
Used netty 4.1.42.Final to be inline with another PR
…le-filter

The master branch contains some important fixes.
Applied quantile filtering while writing metrics into socket.
Corrected formating.
Reverted netty jar version changes.
Appended the new line at the end of files which removed while restoring
previous changes from repo.
@podile
Copy link
Author

podile commented Mar 15, 2020

This PR fixes the issue #77

@eperott
Copy link

eperott commented Mar 30, 2020

Would it be possible to reproduce the issue in a unit test? That way it would be easy to verify the fix, and to make sure that the issue is not re-introduced in a future refactoring.

Also, your PR contain a range of different changes. Perhaps it will be easier for maintainers to review/merge your PR if you can extract your fix into a separate branch before you create the PR.

@podile
Copy link
Author

podile commented Apr 9, 2020

Would it be possible to reproduce the issue in a unit test? That way it would be easy to verify the fix, and to make sure that the issue is not re-introduced in a future refactoring.

Also, your PR contain a range of different changes. Perhaps it will be easier for maintainers to review/merge your PR if you can extract your fix into a separate branch before you create the PR.

Lot of mocking required to initialize and call the method reconcileMBeans of the class JMXHarvester as the constructor itself triggering lot of functionality through metadataFactory. Since the fix I made is very simple and straight forward,I would prefer not to add any test case in this particular case.

Sorry for the inconvenience made by the rage of changes in my PR, I pulled all merged changes form zegelin/cassandra-exporter as it contains important fixes. I will find a better way next time.

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