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

Remove unsupported Version.V_5_* #32937

Merged
merged 18 commits into from
Aug 24, 2018
Merged

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Aug 17, 2018

This change removes the es 5x version constants and their usages.

This change removes the es 5x version constants and their usages.
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

Awesome! I left some comments.

if (res.isEmpty() && nextMajorVersion == 6) {
// bwc for v5
return Version.fromString("5.6.11")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since v7 is not released VersionCollection tries to find the last major before v6. I am not happy with this change though so I added a null version in Version.java to reference V5_6_11:
https://github.com/elastic/elasticsearch/pull/32937/files#diff-818e36d28dd050223297d06218747cbe . This line is then parsed by the regex in build.gradle and correctly reference a v5 release.

@@ -136,13 +135,13 @@ public void testInitialSearchParamsFields() {
// Test stored_fields for versions that support it
searchRequest = new SearchRequest().source(new SearchSourceBuilder());
searchRequest.source().storedField("_source").storedField("_id");
remoteVersion = Version.fromId(between(Version.V_5_0_0_alpha4_ID, Version.CURRENT.id));
remoteVersion = Version.fromId(between(5000004, Version.CURRENT.id));
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add the version it maps to as a comment?

assertThat(initialSearch(searchRequest, query, remoteVersion).getParameters(), hasEntry("stored_fields", "_source,_id"));

// Test fields for versions that support it
searchRequest = new SearchRequest().source(new SearchSourceBuilder());
searchRequest.source().storedField("_source").storedField("_id");
remoteVersion = Version.fromId(between(2000099, Version.V_5_0_0_alpha4_ID - 1));
remoteVersion = Version.fromId(between(2000099, 5000003));
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@@ -150,13 +150,13 @@ public void testLookupRemoteVersion() throws Exception {
assertTrue(called.get());
called.set(false);
sourceWithMockedRemoteCall(false, ContentType.APPLICATION_JSON, "main/5_0_0_alpha_3.json").lookupRemoteVersion(v -> {
assertEquals(Version.V_5_0_0_alpha3, v);
assertEquals(Version.fromId(5000003), v);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment about the version it maps to?

called.set(true);
});
assertTrue(called.get());
called.set(false);
sourceWithMockedRemoteCall(false, ContentType.APPLICATION_JSON, "main/with_unknown_fields.json").lookupRemoteVersion(v -> {
assertEquals(Version.V_5_0_0_alpha3, v);
assertEquals(Version.fromId(5000003), v);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

out.writeBoolean(settings.lenient());
if (out.getVersion().onOrAfter(Version.V_5_1_1)) {
out.writeBoolean(lenientSet);
Copy link
Contributor

Choose a reason for hiding this comment

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

keep this one?

@@ -188,7 +188,7 @@ public long getNumberOfTasksOnNode(String nodeId, String taskName) {

@Override
public Version getMinimalSupportedVersion() {
return Version.V_5_4_0;
return Version.V_6_0_0_alpha1;
Copy link
Contributor

Choose a reason for hiding this comment

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

return Version.CURRENT.minimumCompatibilityVersion(); ?

@@ -82,7 +82,7 @@ public void testGetForUpdateWithParentField() throws IOException {
Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put("index.version.created", Version.V_5_6_0) // for parent field mapper
.put("index.version.created", Version.V_6_0_0) // for parent field mapper
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought _parent wouldn't be allowed on 6.0?

// no model changes between 5.5.0 and 6.3.0.
private Version minVersion = Version.V_5_5_0;
// value for min_version.
private Version minVersion = Version.V_6_3_0;
Copy link
Contributor

Choose a reason for hiding this comment

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

why not 6.0.0 or Version.CURRENT.minCompatibilityVersion()?

// no model changes between 5.5.0 and 6.3.0.
private Version minVersion = Version.V_5_5_0;
// value for min_version.
private Version minVersion = Version.V_6_3_0;
Copy link
Contributor

Choose a reason for hiding this comment

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

why not 6.0.0? I suspect it says 6.3.0 just because it was the latest version at the time this comment was written

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The comment is truncated, it says:

// Stored snapshot documents created prior to 6.3.0 will have no 
// value for min_version.

I'll put it on a single line.

* DO NOT USE: We need to keep this version around until we release the first
* version of 7. The list of supported versions is extracted from this file with a regex (see build.gradle)
* but v7 is not released yet so we search the latest release before v6.
**/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to keep this version around to make VersionCollection happy but that doesn't sound right. The mixed cluster tests still try to spawn v5.6 nodes where I'd expect 6x and 7x only. @rjernst @hub-cap can you take a look ?

Copy link
Member

Choose a reason for hiding this comment

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

On master there should not be any uses of 5.6.11 by the build, since we do not support any kind of upgrade between the two (rolling or full cluster restart). Can you provide a gist with the failure from the gradle side when this constant is not present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It fails with:

* Where:
Build file '/Users/jimczi/Projects/elasticsearch/build.gradle' line: 111

* What went wrong:
A problem occurred evaluating root project 'elasticsearch'.
> java.util.NoSuchElementException (no error message)

The NoSuchElementException is thrown when VersionCollection ctr calls getHighestPreviousMinor.

Copy link
Member

@rjernst rjernst Aug 17, 2018

Choose a reason for hiding this comment

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

Can you run it with --stacktrace and paste a gist here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure:

* Where:
Build file '/Users/jimczi/Projects/elasticsearch/build.gradle' line: 111

* What went wrong:
A problem occurred evaluating root project 'elasticsearch'.
> java.util.NoSuchElementException (no error message)

* Try:
Run with --info or --debug option to get more log output. Run with --scan to get full insights.

* Exception is:
org.gradle.api.GradleScriptException: A problem occurred evaluating root project 'elasticsearch'.
        at org.gradle.groovy.scripts.internal.DefaultScriptRunnerFactory$ScriptRunnerImpl.run(DefaultScriptRunnerFactory.java:92)
        at org.gradle.configuration.DefaultScriptPluginFactory$ScriptPluginImpl$2.run(DefaultScriptPluginFactory.java:204)
        at org.gradle.configuration.ProjectScriptTarget.addConfiguration(ProjectScriptTarget.java:77)
        at org.gradle.configuration.DefaultScriptPluginFactory$ScriptPluginImpl.apply(DefaultScriptPluginFactory.java:209)
        at org.gradle.configuration.BuildOperationScriptPlugin$1.run(BuildOperationScriptPlugin.java:61)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:300)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:292)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:174)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:90)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.configuration.BuildOperationScriptPlugin.apply(BuildOperationScriptPlugin.java:58)
        at org.gradle.configuration.project.BuildScriptProcessor.execute(BuildScriptProcessor.java:41)
        at org.gradle.configuration.project.BuildScriptProcessor.execute(BuildScriptProcessor.java:26)
        at org.gradle.configuration.project.ConfigureActionsProjectEvaluator.evaluate(ConfigureActionsProjectEvaluator.java:34)
        at org.gradle.configuration.project.LifecycleProjectEvaluator$EvaluateProject.run(LifecycleProjectEvaluator.java:105)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:300)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:292)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:174)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:90)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.configuration.project.LifecycleProjectEvaluator.evaluate(LifecycleProjectEvaluator.java:68)
        at org.gradle.api.internal.project.DefaultProject.evaluate(DefaultProject.java:682)
        at org.gradle.api.internal.project.DefaultProject.evaluate(DefaultProject.java:138)
        at org.gradle.execution.TaskPathProjectEvaluator.configure(TaskPathProjectEvaluator.java:35)
        at org.gradle.execution.TaskPathProjectEvaluator.configureHierarchy(TaskPathProjectEvaluator.java:60)
        at org.gradle.configuration.DefaultBuildConfigurer.configure(DefaultBuildConfigurer.java:41)
        at org.gradle.initialization.DefaultGradleLauncher$ConfigureBuild.run(DefaultGradleLauncher.java:266)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:300)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:292)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:174)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:90)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.initialization.DefaultGradleLauncher.configureBuild(DefaultGradleLauncher.java:179)
        at org.gradle.initialization.DefaultGradleLauncher.doBuildStages(DefaultGradleLauncher.java:138)
        at org.gradle.initialization.DefaultGradleLauncher.executeTasks(DefaultGradleLauncher.java:121)
        at org.gradle.internal.invocation.GradleBuildController$1.call(GradleBuildController.java:77)
        at org.gradle.internal.invocation.GradleBuildController$1.call(GradleBuildController.java:74)
        at org.gradle.internal.work.DefaultWorkerLeaseService.withLocks(DefaultWorkerLeaseService.java:152)
        at org.gradle.internal.work.StopShieldingWorkerLeaseService.withLocks(StopShieldingWorkerLeaseService.java:38)
        at org.gradle.internal.invocation.GradleBuildController.doBuild(GradleBuildController.java:96)
        at org.gradle.internal.invocation.GradleBuildController.run(GradleBuildController.java:74)
        at org.gradle.tooling.internal.provider.ExecuteBuildActionRunner.run(ExecuteBuildActionRunner.java:28)
        at org.gradle.launcher.exec.ChainingBuildActionRunner.run(ChainingBuildActionRunner.java:35)
        at org.gradle.tooling.internal.provider.ValidatingBuildActionRunner.run(ValidatingBuildActionRunner.java:32)
        at org.gradle.launcher.exec.RunAsBuildOperationBuildActionRunner$3.run(RunAsBuildOperationBuildActionRunner.java:47)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:300)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor$RunnableBuildOperationWorker.execute(DefaultBuildOperationExecutor.java:292)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.execute(DefaultBuildOperationExecutor.java:174)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:90)
        at org.gradle.internal.operations.DelegatingBuildOperationExecutor.run(DelegatingBuildOperationExecutor.java:31)
        at org.gradle.launcher.exec.RunAsBuildOperationBuildActionRunner.run(RunAsBuildOperationBuildActionRunner.java:43)
        at org.gradle.tooling.internal.provider.SubscribableBuildActionRunner.run(SubscribableBuildActionRunner.java:51)
        at org.gradle.launcher.exec.InProcessBuildActionExecuter$1.transform(InProcessBuildActionExecuter.java:50)
        at org.gradle.launcher.exec.InProcessBuildActionExecuter$1.transform(InProcessBuildActionExecuter.java:46)
        at org.gradle.composite.internal.DefaultRootBuildState.run(DefaultRootBuildState.java:74)
        at org.gradle.launcher.exec.InProcessBuildActionExecuter.execute(InProcessBuildActionExecuter.java:46)
        at org.gradle.launcher.exec.InProcessBuildActionExecuter.execute(InProcessBuildActionExecuter.java:32)
        at org.gradle.launcher.exec.BuildTreeScopeBuildActionExecuter.execute(BuildTreeScopeBuildActionExecuter.java:39)
        at org.gradle.launcher.exec.BuildTreeScopeBuildActionExecuter.execute(BuildTreeScopeBuildActionExecuter.java:25)
        at org.gradle.tooling.internal.provider.ContinuousBuildActionExecuter.execute(ContinuousBuildActionExecuter.java:80)
        at org.gradle.tooling.internal.provider.ContinuousBuildActionExecuter.execute(ContinuousBuildActionExecuter.java:53)
        at org.gradle.tooling.internal.provider.ServicesSetupBuildActionExecuter.execute(ServicesSetupBuildActionExecuter.java:62)
        at org.gradle.tooling.internal.provider.ServicesSetupBuildActionExecuter.execute(ServicesSetupBuildActionExecuter.java:34)
        at org.gradle.tooling.internal.provider.GradleThreadBuildActionExecuter.execute(GradleThreadBuildActionExecuter.java:36)
        at org.gradle.tooling.internal.provider.GradleThreadBuildActionExecuter.execute(GradleThreadBuildActionExecuter.java:25)
        at org.gradle.tooling.internal.provider.ParallelismConfigurationBuildActionExecuter.execute(ParallelismConfigurationBuildActionExecuter.java:43)
        at org.gradle.tooling.internal.provider.ParallelismConfigurationBuildActionExecuter.execute(ParallelismConfigurationBuildActionExecuter.java:29)
        at org.gradle.tooling.internal.provider.StartParamsValidatingActionExecuter.execute(StartParamsValidatingActionExecuter.java:59)
        at org.gradle.tooling.internal.provider.StartParamsValidatingActionExecuter.execute(StartParamsValidatingActionExecuter.java:31)
        at org.gradle.tooling.internal.provider.SessionFailureReportingActionExecuter.execute(SessionFailureReportingActionExecuter.java:59)
        at org.gradle.tooling.internal.provider.SessionFailureReportingActionExecuter.execute(SessionFailureReportingActionExecuter.java:44)
        at org.gradle.tooling.internal.provider.SetupLoggingActionExecuter.execute(SetupLoggingActionExecuter.java:46)
        at org.gradle.tooling.internal.provider.SetupLoggingActionExecuter.execute(SetupLoggingActionExecuter.java:30)
        at org.gradle.launcher.daemon.server.exec.ExecuteBuild.doBuild(ExecuteBuild.java:67)
        at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.WatchForDisconnection.execute(WatchForDisconnection.java:37)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.ResetDeprecationLogger.execute(ResetDeprecationLogger.java:26)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.RequestStopIfSingleUsedDaemon.execute(RequestStopIfSingleUsedDaemon.java:34)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:74)
        at org.gradle.launcher.daemon.server.exec.ForwardClientInput$2.call(ForwardClientInput.java:72)
        at org.gradle.util.Swapper.swap(Swapper.java:38)
        at org.gradle.launcher.daemon.server.exec.ForwardClientInput.execute(ForwardClientInput.java:72)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.LogAndCheckHealth.execute(LogAndCheckHealth.java:50)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.LogToClient.doBuild(LogToClient.java:62)
        at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.EstablishBuildEnvironment.doBuild(EstablishBuildEnvironment.java:82)
        at org.gradle.launcher.daemon.server.exec.BuildCommandOnly.execute(BuildCommandOnly.java:36)
        at org.gradle.launcher.daemon.server.api.DaemonCommandExecution.proceed(DaemonCommandExecution.java:122)
        at org.gradle.launcher.daemon.server.exec.StartBuildOrRespondWithBusy$1.run(StartBuildOrRespondWithBusy.java:50)
        at org.gradle.launcher.daemon.server.DaemonStateCoordinator$1.run(DaemonStateCoordinator.java:295)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:63)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:46)
        at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:55)
Caused by: java.util.NoSuchElementException
        at java_util_SortedSet$last.call(Unknown Source)
        at org.elasticsearch.gradle.VersionCollection.getHighestPreviousMinor(VersionCollection.groovy:296)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at org.elasticsearch.gradle.VersionCollection.<init>(VersionCollection.groovy:142)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
        at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at build_5ys12t32hufwlh0gq31qqnlig.run(/Users/jimczi/Projects/elasticsearch/build.gradle:111)
        at org.gradle.groovy.scripts.internal.DefaultScriptRunnerFactory$ScriptRunnerImpl.run(DefaultScriptRunnerFactory.java:90)
        ... 99 more
```

Copy link
Member

Choose a reason for hiding this comment

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

I think we just want to set maintenanceBugfixSnapshot to null in this case, because the outer block that is within is for master, where it does not make sense to reference the maintenance version (eg 5.6.11) ever, since there should be no uses by bwc tests. Can you try changing line 141-143 of VersionCollection to just set it to null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, it seems to work so I pushed the changes, let's see if the build passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mixed cluster tests starts a v5.6 node which fails the handshake with the other v6 nodes. I don't understand why it uses a 5x node since all references to v5 are now removed. Did I miss something ?

Copy link
Member

Choose a reason for hiding this comment

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

This does seem odd. I think @hub-cap needs to take a look at this too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry the mixed cluster tests are fine (they start a mix of v6.5 and v7 nodes now) but they fail the handshake. I talked to @s1monw which pointed out that we also need to force the minimum compatibility of version 6 to the removed v5.6 series. I pushed the fix and all tests pass locally :)

@colings86 colings86 added the :Search/Search Search-related issues that do not fall into other categories label Aug 20, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@jimczi jimczi added the v7.0.0 label Aug 21, 2018
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

VersionCollection might need more eyes, the rest LGTM.

@jimczi
Copy link
Contributor Author

jimczi commented Aug 23, 2018

Thanks @jpountz , @s1monw could you take a look at the Version and VersionCollection changes ?

Copy link
Contributor

@s1monw s1monw left a comment

Choose a reason for hiding this comment

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

left 2 question LGTM otherwise

@@ -629,7 +475,10 @@ public boolean isRelease() {
}
assert field.getName().matches("V(_\\d+)+(_(alpha|beta|rc)\\d+)?") : field.getName();
try {
versions.add(((Version) field.get(null)));
// special case for V_5_6_11
if (field.get(null) != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure I am following can you leave a comment what this actually does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a leftover, I removed it thanks

@@ -140,7 +140,11 @@ class VersionCollection {
}
// caveat 0 - now dip back 2 versions to get the last supported snapshot version of the line
Version highestMinor = getHighestPreviousMinor(currentVersion.major - 1)
maintenanceBugfixSnapshot = replaceAsSnapshot(highestMinor)
if (highestMinor == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a pre-existing bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Though maintenanceBugfixSnapshot should always be null in this case so I pushed 40d70b4

@jimczi jimczi merged commit f4e9729 into elastic:master Aug 24, 2018
@jimczi jimczi deleted the version5_removal branch August 24, 2018 07:51
jasontedor added a commit that referenced this pull request Aug 24, 2018
* master:
  Add hook to skip asserting x-content equivalence (#33114)
  Muted testListenersThrowingExceptionsDoNotCauseOtherListenersToBeSkipped
  [Rollup] Move getMetadata() methods out of rollup config objects (#32579)
  Muted testEmptyAuthorizedIndicesSearchForAllDisallowNoIndices
  Update Google Cloud Storage Library for Java (#32940)
  Remove unsupported Version.V_5_* (#32937)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 24, 2018
* ccr:
  Add hook to skip asserting x-content equivalence (elastic#33114)
  Muted testListenersThrowingExceptionsDoNotCauseOtherListenersToBeSkipped
  [Rollup] Move getMetadata() methods out of rollup config objects (elastic#32579)
  fixed not returning response instance
  Muted testEmptyAuthorizedIndicesSearchForAllDisallowNoIndices
  Update Google Cloud Storage Library for Java (elastic#32940)
  Remove unsupported Version.V_5_* (elastic#32937)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 24, 2018
* ccr: (71 commits)
  Make CCR QA tests build again (elastic#33113)
  Add hook to skip asserting x-content equivalence (elastic#33114)
  Muted testListenersThrowingExceptionsDoNotCauseOtherListenersToBeSkipped
  [Rollup] Move getMetadata() methods out of rollup config objects (elastic#32579)
  fixed not returning response instance
  Muted testEmptyAuthorizedIndicesSearchForAllDisallowNoIndices
  Update Google Cloud Storage Library for Java (elastic#32940)
  Remove unsupported Version.V_5_* (elastic#32937)
  Required changes after merging in master branch.
  [DOCS] Add docs for Application Privileges (elastic#32635)
  Add versions 5.6.12 and 6.4.1
  Do NOT allow termvectors on nested fields (elastic#32728)
  [Rollup] Return empty response when aggs are missing (elastic#32796)
  [TEST] Add some ACL yaml tests for Rollup (elastic#33035)
  Move non duplicated actions back into xpack core (elastic#32952)
  Test fix - GraphExploreResponseTests should not randomise array elements Closes elastic#33086
  Use `addIfAbsent` instead of checking if an element is contained
  TESTS: Fix Random Fail in MockTcpTransportTests (elastic#33061)
  HLRC: Fix Compile Error From Missing Throws (elastic#33083)
  [DOCS] Remove reload password from docs cf. elastic#32889
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories >upgrade v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants