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

GroupProperty defaulting not working properly when programmatic configuration is used #6174

Closed
jerrinot opened this issue Sep 10, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@jerrinot
Copy link
Contributor

commented Sep 10, 2015

Example:
Properties IO_INPUT_THREAD_COUNT and IO_OUTPUT_THREAD_COUNT are configurable. The both defaults to IO_THREAD_COUNT

    IO_THREAD_COUNT("hazelcast.io.thread.count", 3),
    IO_INPUT_THREAD_COUNT("hazelcast.io.input.thread.count", IO_THREAD_COUNT),
    IO_OUTPUT_THREAD_COUNT("hazelcast.io.output.thread.count", IO_THREAD_COUNT),

This seems to work fine. However when I change the IO_THREAD_COUNT property then I'd intuitively expect the new value will be reflected in dependent properties.

In other words: I'd expect following test to pass:

@Test
    public void testInheritance_whenDefaultIsChangedProgramatically_thenDependentPropertyIsChangedToo() {
        Config config = new Config();
        config.setProperty(GroupProperty.IO_THREAD_COUNT, "1");
        HazelcastInstance instance = createHazelcastInstance(config);

        Node node = getNode(instance);
        int inputIOThreadCount = node.getGroupProperties().getInteger(GroupProperty.IO_INPUT_THREAD_COUNT);

        assertEquals(1, inputIOThreadCount);
    }

However the test is failing due the way how properties are resolved. I'm not sure if the issue is solvable with the current GrouProperty design. If not that the defaulting mechanism should be removed from (GroupProperty) as it's misleading.

@jerrinot jerrinot added this to the 3.6 milestone Sep 10, 2015

@Donnerbart

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

We cannot get rid of the default mechanism completely, otherwise most of the properties would have no value at all. We could fix it for inheritance by adding a new field. I can make a PR on the weekend.

Thanks for the finding!

@jerrinot

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2015

it's a pass-by-value vs. pass-by-reference problem.

@Donnerbart

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2015

I know, but it's fixable.

@jerrinot

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2015

great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.