ReindexAll() improvements #2462

Mpdreamz · 2016-12-06T20:25:00Z

ReindexAll() is now composed over the IObservables from ScrollAll()
and BulkAll() taking advantage of both's build in concurrency.

Since the rate at which the scrolls produces far exceeds the rate at which we can consume them through bulks ReindexAll() also implements a producerconsumer rate limiter.
This is controlled by a backPressureFactor which controls the max amplification factor of running scrolls with the safe guard of:

searchSize * maxConcurrency * backPressureFactor >= bulkSize

Otherwise not enough scrolls would be spawned to feed a single bulk
request. Obviously if the scroll observable calls on complete it will
flush out any pending scrolls and feed it to BulkAll().

Mpdreamz · 2016-12-06T20:29:00Z

remote: Resolving deltas: 100% (10/10), completed with 10 local objects.
remote: Unexpected system error after push was received.
remote: These changes may not be reflected on github.com!
remote: Your unique error code: df69348cdee013b3cc32ec98aa672431

This PR seems to be missing my force push :( never seen this before

Mpdreamz · 2016-12-06T20:34:51Z

OK that message can happen when github fails to update its caches on serverside commit hook. If you are looking at a diff on github.com that has limiter misspelled you are not reviewing the right commit. Please review locally. (commit list lists the correct sha numbers on here though).

russcam

Looks good, just a few minor nitpicks 😃

Some of the types introduced are not in the Nest namespace
Reindex API has a few warnings with xml comments
Reindex API does not adhere to Tests.CodeStandards.ElasticClientStandards.ConsistentFluentParameterNames

russcam · 2016-12-07T01:38:12Z

src/Nest/CommonAbstractions/Infer/Inferrer.cs

@@ -31,6 +34,7 @@ public Inferrer(IConnectionSettingsValues connectionSettings)
 			this.CreateMultiHitDelegates = new ConcurrentDictionary<Type, Action<MultiGetHitJsonConverter.MultiHitTuple, JsonSerializer, ICollection<IMultiGetHit<object>>>>();
 			this.CreateSearchResponseDelegates = new ConcurrentDictionary<Type, Action<MultiSearchResponseJsonConverter.SearchHitTuple, JsonSerializer, IDictionary<string, object>>>();
 		}
+		public string Resolve(IUrlParameter urlParameter) => urlParameter.GetString(this._connectionSettings);


russcam · 2016-12-07T01:38:40Z

src/Nest/CommonAbstractions/Reactive/CoordinatedRequestObserverBase.cs

@@ -1,8 +1,15 @@
 using System;

-
-namespace Nest
+namespace Nest.CommonAbstractions.Reactive


namespace should be Nest

russcam · 2016-12-07T01:38:52Z

src/Nest/CommonAbstractions/Reactive/GetEnumerator.cs

+using System.Collections.Generic;
+using System.Threading;
+
+namespace Nest.CommonAbstractions.Reactive


namespace should be Nest

russcam · 2016-12-07T01:39:46Z

src/Nest/CommonAbstractions/Reactive/PartitionHelper.cs

+using System.Collections.Generic;
+using System.Linq;
+
+namespace Nest.CommonAbstractions.Reactive


namespace should be Nest

russcam · 2016-12-07T01:40:02Z

src/Nest/CommonAbstractions/Reactive/ProducerConsumerBackPressure.cs

+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Nest.CommonAbstractions.Reactive


namespace should be Nest

russcam · 2016-12-07T01:40:50Z

src/Nest/CommonAbstractions/Reactive/ProducerConsumerBackPressure.cs

+{
+	/// <summary>
+	/// Simple back pressure implementation that makes sure the minimum max concurrency between producer and consumer
+	/// is not amplified by the greedier of the by more then the backPressureFactor


"...greedier of the two..."

russcam · 2016-12-07T01:46:44Z

src/Nest/Document/Multiple/BulkAll/BulkAllRequest.cs

+		/// <see cref="MaxDegreeOfParallelism"/> if set. Not that the consumer has to call <see cref="ProducerConsumerBackPressure.Release"/>
+		/// on the same instance every time it is done.
+		/// <param name="maxConcurrency">The minimum maximum concurrency which would be the bottleneck of the producer consumer pipeline</param>
+		/// <param name="backPressureFactor">The maximum amplification back pressure of the greedier part of the producer consumer pipeline</param>


I think we should point out what the default backPressureFactor is if unspecified.

ReindexAll() is now composed over the IObservables from `ScrollAll()` and `BulkAll()` taking advantage of both's build in concurrency. Since the rate at which the scroll's will come in far exceeds the rate at which we can bulk the `ReindexAll()` also implements a producer consumer rate limitter. This is controlled by a `backPressureFactor` which controls the max amplification factor of running scrolls with the safe guard of: searchSize * maxConcurrency * backPressureFactor >= bulkSize Otherwise not enough scrolls would be spawned to feed a single bulk request. Obviously if the scroll observable calls on complete it will flush out any pending scrolls and feed it to `BulkAll()`.

…lt for backPressureFactor and small documentation touch ups

* ReindexAll() improvements ReindexAll() is now composed over the IObservables from `ScrollAll()` and `BulkAll()` taking advantage of both's build in concurrency. Since the rate at which the scroll's will come in far exceeds the rate at which we can bulk the `ReindexAll()` also implements a producer consumer rate limitter. This is controlled by a `backPressureFactor` which controls the max amplification factor of running scrolls with the safe guard of: searchSize * maxConcurrency * backPressureFactor >= bulkSize Otherwise not enough scrolls would be spawned to feed a single bulk request. Obviously if the scroll observable calls on complete it will flush out any pending scrolls and feed it to `BulkAll()`. * implemented pr review feedback, namespace changes, document the default for backPressureFactor and small documentation touch ups

Mpdreamz · 2016-12-07T08:42:35Z

ported to 5.x but was to eager and did not fix the xml doc warning and the test will do so now on both branches

* ReindexAll() improvements ReindexAll() is now composed over the IObservables from `ScrollAll()` and `BulkAll()` taking advantage of both's build in concurrency. Since the rate at which the scroll's will come in far exceeds the rate at which we can bulk the `ReindexAll()` also implements a producer consumer rate limitter. This is controlled by a `backPressureFactor` which controls the max amplification factor of running scrolls with the safe guard of: searchSize * maxConcurrency * backPressureFactor >= bulkSize Otherwise not enough scrolls would be spawned to feed a single bulk request. Obviously if the scroll observable calls on complete it will flush out any pending scrolls and feed it to `BulkAll()`. * implemented pr review feedback, namespace changes, document the default for backPressureFactor and small documentation touch ups

russcam suggested changes Dec 7, 2016

View reviewed changes

Mpdreamz force-pushed the feature/scroll-all branch from a93e1ca to 43d5e55 Compare December 7, 2016 08:25

Mpdreamz force-pushed the feature/reindex-next-level branch from 6d07c32 to 92ec781 Compare December 7, 2016 08:32

Mpdreamz changed the base branch from feature/scroll-all to master December 7, 2016 08:33

implemented pr review feedback, namespace changes, document the defau…

831881c

…lt for backPressureFactor and small documentation touch ups

Mpdreamz merged commit 3e58310 into master Dec 7, 2016

Mpdreamz added the Port to 5.x label Dec 7, 2016

Mpdreamz deleted the feature/reindex-next-level branch December 7, 2016 08:42

russcam added the v5.0.0 label Dec 13, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ReindexAll() improvements #2462

ReindexAll() improvements #2462

Uh oh!

Mpdreamz commented Dec 6, 2016

Uh oh!

Mpdreamz commented Dec 6, 2016

Uh oh!

Mpdreamz commented Dec 6, 2016 •

edited

Loading

Uh oh!

russcam left a comment

Uh oh!

russcam Dec 7, 2016

Uh oh!

russcam Dec 7, 2016

Uh oh!

russcam Dec 7, 2016

Uh oh!

russcam Dec 7, 2016

Uh oh!

russcam Dec 7, 2016

Uh oh!

russcam Dec 7, 2016

Uh oh!

russcam Dec 7, 2016

Uh oh!

Mpdreamz commented Dec 7, 2016

Uh oh!

Uh oh!

ReindexAll() improvements #2462

ReindexAll() improvements #2462

Uh oh!

Conversation

Mpdreamz commented Dec 6, 2016

Uh oh!

Mpdreamz commented Dec 6, 2016

Uh oh!

Mpdreamz commented Dec 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

russcam left a comment

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

russcam Dec 7, 2016

Choose a reason for hiding this comment

Uh oh!

Mpdreamz commented Dec 7, 2016

Uh oh!

Uh oh!

Mpdreamz commented Dec 6, 2016 •

edited

Loading