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.

Sign up for GitHub

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

Jump to bottom

Make sure NodeJoinController.ElectionCallback is always called from the update cluster state thread #12372

Closed
wants to merge 2 commits into from
Closed

Make sure NodeJoinController.ElectionCallback is always called from the update cluster state thread #12372

wants to merge 2 commits into from

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Jul 21, 2015

This is important for correct handling of the joining thread. This causes assertions to trip in our test runs. See http://build-us-00.elastic.co/job/es_g1gc_master_metal/11653/ as an example

@bleskes
Discovery: make sure NodeJoinController.ElectionCallback is always ca…
f2f1211 
…lled from the update cluster state thread

This is important for correct handling of the joining thread. This causes assertions to trip in our test runs. See http://build-us-00.elastic.co/job/es_g1gc_master_metal/11653/ as an example
@bleskes bleskes added >bug v2.0.0-beta1 review :Distributed/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure labels Jul 21, 2015
martijnvg
martijnvg reviewed Jul 21, 2015
View reviewed changes
core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java
@@ -1307,7 +1306,8 @@ public void start() {
}

private void assertClusterStateThread() {
assert Thread.currentThread().getName().contains(InternalClusterService.UPDATE_THREAD_NAME) : "not called from the cluster state update thread";
assert clusterService instanceof InternalClusterService == false || ((InternalClusterService) clusterService).assertClusterStateThread();
;
Copy link
Member

Choose a reason for hiding this comment

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

redundant semicolon

@martijnvg
Copy link
Member

LGTM

@bleskes bleskes closed this in 5def419 Jul 21, 2015
@bleskes bleskes deleted the node_controler_callback branch July 21, 2015 14:11
rmuir pushed a commit to rmuir/elasticsearch that referenced this pull request Jul 22, 2015
@bleskes @rmuir
Discovery: make sure NodeJoinController.ElectionCallback is always ca…
b81ccba 
…lled from the update cluster state thread

This is important for correct handling of the joining thread. This causes assertions to trip in our test runs. See http://build-us-00.elastic.co/job/es_g1gc_master_metal/11653/ as an example

Closes elastic#12372
rmuir added a commit that referenced this pull request Jul 22, 2015
@rmuir
Refactor pluginservice
4040f19 
Closes #12367

Squashed commit of the following:

commit 9453c41
Merge: 3511a9c 828d8c7
Author: Robert Muir <rmuir@apache.org>
Date:   Wed Jul 22 08:22:41 2015 -0400

    Merge branch 'master' into refactor_pluginservice

commit 3511a9c
Author: Ryan Ernst <ryan@iernst.net>
Date:   Tue Jul 21 21:50:15 2015 -0700

    Remove duplicated constant

commit 4a9b5b4
Author: Ryan Ernst <ryan@iernst.net>
Date:   Tue Jul 21 21:01:57 2015 -0700

    Add check that plugin must specify at least site or jvm

commit 19aef2f
Author: Ryan Ernst <ryan@iernst.net>
Date:   Tue Jul 21 20:52:58 2015 -0700

    Change plugin "plugin" property to "classname"

commit 07ae396
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 23:36:05 2015 -0400

    remove test with no methods

commit 550e73b
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 23:31:58 2015 -0400

    fix loading to use classname

commit 04463ae
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 23:24:19 2015 -0400

    rename to classname

commit 9f3afad
Author: Ryan Ernst <ryan@iernst.net>
Date:   Tue Jul 21 20:18:46 2015 -0700

    moved PluginInfo and refactored parsing from properties file

commit df63ccc
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 23:08:26 2015 -0400

    fix test

commit c7febd8
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 23:03:44 2015 -0400

    remove test

commit 017b341
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 22:58:31 2015 -0400

    fix test

commit c992293
Merge: ad59af4 01ea89a
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 22:37:28 2015 -0400

    Merge branch 'master' into refactor_pluginservice

commit ad59af4
Author: Areek Zillur <areek.zillur@elasticsearch.com>
Date:   Tue Jul 21 19:30:26 2015 -0400

    [TEST] Verify expected number of nodes in cluster before issuing shardStores request

commit f0f5a1e
Author: Lee Hinman <lee@writequit.org>
Date:   Tue Jul 21 11:27:28 2015 -0600

    Ignore EngineClosedException during translog fysnc

    When performing an operation on a primary, the state is captured and the
    operation is performed on the primary shard. The original request is
    then modified to increment the version of the operation as preparation
    for it to be sent to the replicas.

    If the request first fails on the primary during the translog sync
    (because the Engine is already closed due to shadow primaries closing
    the engine on relocation), then the operation is retried on the new primary
    after being modified for the replica shards. It will then fail due to the
    version being incorrect (the document does not yet exist but the request
    expects a version of "1").

    Order of operations:

    - Request is executed against primary
    - Request is modified (version incremented) so it can be sent to replicas
    - Engine's translog is fsync'd if necessary (failing, and throwing an exception)
    - Modified request is retried against new primary

    This change ignores the exception where the engine is already closed
    when syncing the translog (similar to how we ignore exceptions when
    refreshing the shard if the ?refresh=true flag is used).

commit 4ac68bb
Author: Shay Banon <kimchy@gmail.com>
Date:   Tue Jul 21 22:37:29 2015 +0200

    Replica allocator unit tests
    First batch of unit tests to verify the behavior of replica allocator

commit 94609fc
Author: Jason Tedor <jason@tedor.me>
Date:   Tue Jul 21 14:04:46 2015 -0400

    Correctly list blobs in Azure storage to prevent snapshot corruption and do not unnecessarily duplicate Lucene segments in Azure Storage

    This commit addresses an issue that was leading to snapshot corruption for snapshots stored as blobs in Azure Storage.

    The underlying issue is that in cases when multiple snapshots of an index were taken and persisted into Azure Storage, snapshots subsequent
    to the first would repeatedly overwrite the snapshot files. This issue does render useless all snapshots except the final snapshot.

    The root cause of this is due to String concatenation involving null. In particular, to list all of the blobs in a snapshot directory in
    Azure the code would use the method listBlobsByPrefix where the prefix is null. In the listBlobsByPrefix method, the path keyPath + prefix
    is constructed. However, per 5.1.11, 5.4 and 15.18.1 of the Java Language Specification, the reference null is first converted to the string
    "null" before performing the concatenation. This leads to no blobs being returned and therefore the snapshot mechanism would operate as if
    it were writing the first snapshot of the index. The fix is simply to check if prefix is null and handle the concatenation accordingly.

    Upon fixing this issue so that subsequent snapshots would no longer overwrite earlier snapshots, it was discovered that the snapshot metadata
    returned by the listBlobsByPrefix method was not sufficient for the snapshot layer to detect whether or not the Lucene segments had already
    been copied to the Azure storage layer in an earlier snapshot. This led the snapshot layer to unnecessarily duplicate these Lucene segments
    in Azure Storage.

    The root cause of this is due to known behavior in the CloudBlobContainer.getBlockBlobReference method in the Azure API. Namely, this method
    does not fetch blob attributes from Azure. As such, the lengths of all the blobs appeared to the snapshot layer to be of length zero and
    therefore they would compare as not equal to any new blobs that the snapshot layer is going to persist. To remediate this, the method
    CloudBlockBlob.downloadAttributes must be invoked. This will fetch the attributes from Azure Storage so that a proper comparison of the
    blobs can be performed.

    Closes elastic/elasticsearch-cloud-azure#51, closes elastic/elasticsearch-cloud-azure#99

commit cf1d481
Author: Lee Hinman <lee@writequit.org>
Date:   Mon Jul 20 08:41:55 2015 -0600

    Unit tests for `nodesAndVersions` on shared filesystems

    With the `recover_on_any_node` setting, these unit tests check that the
    correct node list and versions are returned.

commit 3c27cc3
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 14:15:59 2015 -0400

    don't fail junit4 integration tests if there are no tests.

    instead fail the failsafe plugin, which means the external cluster will still get shut down

commit 95d2756
Author: Alexander Reelsen <alexander@reelsen.net>
Date:   Tue Jul 21 17:16:53 2015 +0200

    Testing: Fix help displaying tests under windows

    The help files are using a unix based file separator, where as
    the test relies on the help being based on the file system separator.

    This commit fixes the test to remove all `\r` characters before
    comparing strings.

    The test has also been moved into its own CliToolTestCase, as it does
    not need to be an integration test.

commit 944f06e
Author: Clinton Gormley <clint@traveljury.com>
Date:   Tue Jul 21 18:04:52 2015 +0200

    Refactored check_license_and_sha.pl to accept a license dir and package path

    In preparation for the move to building the core zip, tar.gz, rpm, and deb as separate modules, refactored check_license_and_sha.pl to:

    * accept a license dir and path to the package to check on the command line
    * to be able to extract zip, tar.gz, deb, and rpm
    * all packages except rpm will work on Windows

commit 2585431
Author: Chris Earle <pickypg@users.noreply.github.com>
Date:   Tue Jul 21 08:35:28 2015 -0700

    Updating breaking changes

    - field names cannot be mapped with `.` in them
    - fixed asciidoc issue where the list was not recognized as a list

commit de299b9
Author: Shay Banon <kimchy@gmail.com>
Date:   Tue Jul 21 13:27:52 2015 +0200

    Replace primaryPostAllocated flag and use UnassignedInfo
    There is no need to maintain additional state as to if a primary was allocated post api creation on the index routing table, we hold all this information already in the UnassignedInfo class.
    closes #12374

commit 43080bf
Author: Alexander Reelsen <alexander@reelsen.net>
Date:   Tue Jul 21 15:45:05 2015 +0200

    PluginManager: Fix bin/plugin calls in scripts/bats test

    The release and smoke test python scripts used to install
    plugins in the old fashion.

    Also the BATS testing suite installed/removed plugins in that
    way. Here the marvel tests have been removed, as marvel currently
    does not work with the master branch.

    In addition documentation has been updated as well, where it was
    still missing.

commit b81ccba
Author: Boaz Leskes <b.leskes@gmail.com>
Date:   Tue Jul 21 11:37:50 2015 +0200

    Discovery: make sure NodeJoinController.ElectionCallback is always called from the update cluster state thread

    This is important for correct handling of the joining thread. This causes assertions to trip in our test runs. See http://build-us-00.elastic.co/job/es_g1gc_master_metal/11653/ as an example

    Closes #12372

commit 3318537
Author: Boaz Leskes <b.leskes@gmail.com>
Date:   Tue Jul 21 15:54:36 2015 +0200

    Remove left over no commit from TransportReplicationAction

    It asks to double check thread pool rejection. I did and don't see problems with it.

commit e572493
Author: Alexander Reelsen <alexander@reelsen.net>
Date:   Tue Jul 21 15:31:57 2015 +0200

    CliTool: Various PluginManager fixes

    The new plugin manager parser was not called correctly in the scripts.
    In addition the plugin manager now creates a plugins/ directory in case
    it does not exist.

    Also the integration tests called the plugin manager in the deprecated way.

commit 7a815a3
Author: Alexander Reelsen <alexander@reelsen.net>
Date:   Tue Jul 21 13:54:18 2015 +0200

    CLITool: Port PluginManager to use CLITool

    In order to unify the handling and reuse the CLITool infrastructure
    the plugin manager should make use of this as well.

    This obsolets the -i and --install options but requires the user
    to use `install` as the first argument of the CLI.

    This is basically just a port of the existing functionality, which
    is also the reason why this is not a refactoring of the plugin manager,
    which will come in a separate commit.

commit 7f171eb
Author: Martijn van Groningen <martijn.v.groningen@gmail.com>
Date:   Tue Jul 21 10:44:21 2015 +0200

    Remove custom execute local logic in TransportSingleShardAction and TransportInstanceSingleOperationAction and rely on transport service to execute locally. (forking thread etc.)

    Change TransportInstanceSingleOperationAction to have shardActionHandler to, so we can execute locally without endless spinning.

commit 0f38e3e
Author: Ryan Ernst <ryan@iernst.net>
Date:   Tue Jul 21 17:36:12 2015 -0700

    More readMetadata tests and pickiness

commit 880b472
Author: Ryan Ernst <ryan@iernst.net>
Date:   Tue Jul 21 14:42:09 2015 -0700

    Started unit tests for plugin service

commit cd7c8dd
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 07:21:07 2015 -0400

    fix tests

commit 673454f
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Jul 21 06:58:25 2015 -0400

    refactor pluginservice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
>bug :Distributed/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure v2.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bleskes @martijnvg @clintongormley