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

Document discovery.zen.commit_timeout treats negative values like 0. Issue 36632 #37570

Closed
wants to merge 1 commit into from

Conversation

cshtdd
Copy link

@cshtdd cshtdd commented Jan 17, 2019

The scope of this PR is document the discovery.zen.commit_timeout treats negative values as 0.

A different PR will be required to update the Docs.

Supporting Research

This test proves the setting can be negative

public void testReadsNegativeCommitTimeout() {
    Settings settings = Settings.builder()
        .put(DiscoverySettings.COMMIT_TIMEOUT_SETTING.getKey(), "-10s")
        .build();

    TimeValue timeoutValue = DiscoverySettings.COMMIT_TIMEOUT_SETTING.get(settings);

    assertEquals(timeoutValue, new TimeValue(-10, TimeUnit.SECONDS));
}

However, the usage of DiscoverySettings.getCommitTimeout() leads down to this

private boolean doAcquireSharedNanos(int arg, long nanosTimeout) throws InterruptedException {
        if (nanosTimeout <= 0L)
            return false;
        ...
}

@cshtdd cshtdd changed the title Document discovery.zen.commit_timeout treats negative values like 0. Issue #36632 Document discovery.zen.commit_timeout treats negative values like 0. Issue 36632 Jan 17, 2019
@danielmitterdorfer danielmitterdorfer added >docs General docs changes :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Jan 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this setting is user-facing, I think the important documentation is the user-facing reference manual, i.e. docs/reference/modules/discovery/zen.asciidoc. Note that this file does not exist any more in master, reflecting that this setting will not be relevant in versions ≥7.0. You should make a PR against the 6.x branch.

@javanna
Copy link
Member

javanna commented Feb 25, 2019

hi @camilin87 would you mind opening a new PR against the 6.7 branch please? Like @DaveCTurner said above, this setting is not relevant anymore in master. Thanks!

@javanna javanna closed this Feb 25, 2019
@cshtdd
Copy link
Author

cshtdd commented Feb 25, 2019

No problem. I've been pretty busy for the last weeks. And I don't foresee that changing. I'll do it as soon as possible.

@javanna
Copy link
Member

javanna commented Feb 25, 2019

sounds good thanks @camilin87 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >docs General docs changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants