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

Add 7.1 version constant to 7.x branch #38513

Merged
merged 27 commits into from
Feb 7, 2019

Conversation

jasontedor
Copy link
Member

This commit adds the 7.1 version constant to the 7.x branch.

This commit adds the 7.1 version constant to the 7.x branch.
@jasontedor
Copy link
Member Author

This fails in VersionCollection:

unreleased.add(getLatestVersionByKey(groupByMinor, greatestMinor - 2));

The assumptions about the unreleased versions is incorrect.

@jasontedor jasontedor added >non-issue :Delivery/Build Build or test infrastructure v7.2.0 labels Feb 6, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@alpar-t
Copy link
Contributor

alpar-t commented Feb 6, 2019

We need to make changes to account for the platform specific changes for bwc builds

@alpar-t
Copy link
Contributor

alpar-t commented Feb 7, 2019

full cluster restart still fails with some seeds:

  2> REPRODUCE WITH: ./gradlew :qa:full-cluster-restart:v7.0.0#oldClusterTestRunner -Dtests.seed=988DCA3188D1E86E -Dtests.class=org.elasticsearch.upgrades.FullClusterRestartIT -Dtests.method="testRecovery" -Dtests.security.manager=true -Dtests.locale=en-FM -Dtests.timezone=Asia/Pontianak -Dcompiler.java=11 -Druntime.java=11
FAILURE 1.16s | FullClusterRestartIT.testRecovery <<< FAILURES!
   > Throwable #1: java.lang.AssertionError: expected:<297> but was:<292>
   >    at __randomizedtesting.SeedInfo.seed([988DCA3188D1E86E:597DB39DA58122C9]:0)
   >    at org.elasticsearch.upgrades.FullClusterRestartIT.assertTotalHits(FullClusterRestartIT.java:667)
   >    at org.elasticsearch.upgrades.FullClusterRestartIT.testRecovery(FullClusterRestartIT.java:777)
   >    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 java.base/java.lang.reflect.Method.invoke(Method.java:566)
   >    at java.base/java.lang.Thread.run(Thread.java:834)

@alpar-t
Copy link
Contributor

alpar-t commented Feb 7, 2019

@elasticmachine run elasticsearch-ci/default-distro

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

I think this is right, but I have a concern around isRunningAgainstAncientCluster

@@ -863,7 +845,7 @@ public void testRecovery() throws Exception {
*/
public void testSnapshotRestore() throws IOException {
int count;
if (isRunningAgainstOldCluster()) {
if (isRunningAgainstOldCluster() && getOldClusterVersion().major < 8) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what circumstances would the old cluster ever be 8.x (or higher) for tests running from the 7.x branch?

Copy link
Contributor

Choose a reason for hiding this comment

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

It won't for this branch, but as we bump the versions I think we could get there.

@@ -63,6 +63,10 @@ public final boolean isRunningAgainstOldCluster() {

private final Version oldClusterVersion = Version.fromString(System.getProperty("tests.old_cluster_version"));

public final boolean isRunningAgainstAncientCluster() {
return oldClusterVersion.before(Version.V_7_0_0);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this method should check runningAgainstOldCluster (if it's really about which cluster we're running against) or be called something more like isOldClusterAncient (if it's just check the version of the old cluster).

Otherwise the name implies that the behaviour is like isRunningAgainstOldCluster, but it is not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, good catch @tvernum.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason this works most of the time is because most of the invocations to it are already guarded by if (isRunningAgainstOldVersion()).

@@ -1132,7 +1121,7 @@ private void checkSnapshot(String snapshotName, int count, Version tookOnVersion

// In 7.0, type names are no longer returned by default in get index template requests.
// We therefore use the deprecated typed APIs when running against the current version.
if (isRunningAgainstOldCluster() == false) {
if (isRunningAgainstAncientCluster() == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the behaviour of isRunningAgainstOldCluster (but see my comment there about the name vs behaviour), this check seems wrong.

If the old cluster is 7.0.0 then isRunningAgainstAncientCluster will be false, and INCLUDE_TYPE_NAME_PARAMETER will be true, but I think we only want that if old version is pre-7 and we're running on the new version.

Similarly if we're testing against 6.7.x the parameter will be added regardless of which cluster we're running against (old or new), which is not how this test worked before.

But maybe I'm confused about why we want this parameter.

Suggested change
if (isRunningAgainstAncientCluster() == false) {
if (isRunningAgainstOldCluster() == false && getOldClusterVersion().major < 7) {

logger.info("Indexing {} random documents", count);
for (int i = 0; i < count; i++) {
logger.debug("Indexing document [{}]", i);
Request createDocument = new Request("POST", "/" + index + "/doc/" + i);
Request createDocument = new Request("POST", "/" + index + "/" + type + "/" + i);
if (isRunningAgainstAncientCluster() == false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think is going to get caught by isRunningAgainstAncientCluster not checking which cluster we run on.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think all invocations (on mobile, so only going from memory from late last night) of this method are run against the old cluster (by a guard higher up the stack). We should fix the method though.

jkakavas and others added 4 commits February 7, 2019 18:28
)

The clock resolution changed from jdk8 to jdk10, hence the
test is passing in jdk8 but failing in jdk10.  Scheduled
events are serialised and deserialised with millisecond
precision, making test fail in jdk 10 and higher.

Fixes a problem introduced by elastic#38415 and the fix is
identical to the one that was made in elastic#38405.
@alpar-t
Copy link
Contributor

alpar-t commented Feb 7, 2019

@elasticmachine run elasticsearch-ci/1

@Tim-Brooks
Copy link
Contributor

This finicky test (#38489) failed on CI/1. I will push an AwaitsFix if more changes need to be made to this PR (based on other failures).

martijnvg and others added 6 commits February 7, 2019 18:53
whether `types` field should be used should be decided later,
also it looks like this test was intented to be muted.
('tests.rest.blacklist' system property was in the wrong place)
@jasontedor jasontedor merged commit fdf6b3f into elastic:7.x Feb 7, 2019
@jasontedor jasontedor deleted the add-seven-dot-one branch February 7, 2019 21:32
tvernum added a commit that referenced this pull request Feb 8, 2019
When we are preparing to release a major version the rules around
"unreleased" versions and branches get a bit more complex.

This change implements the following rules:

- If the tip version on the previous major is a .0 (e.g. 6.7.0) then
  the tip of the minor before that (e.g. 6.6.1) must be unreleased.
  (This is because 6.7.0 would be "staged" in preparation for release,
  but 6.6.1 would be open for bug fixes on the release 6.6.x line)
  (in VersionCollection & VersionUtils)

- The "major.x" branch (if it exists) will always point to the latest
  minor in that series. Anything that is not the latest minor, must
  therefore be on a the "major.minor" branch
  For example, if v7.1.0 exists then the "7.x" branch must be 7.1.0,
  and 7.0.0 must be on the "7.0" branch
  (in VersionCollection)

- Special logic so that the 7.0.0 build knows that we do not plan
   to have any 6.x releases after the 6.7 series
 
Backport of: #38593 
Partial Backport of: #38513

Co-authored-by: Jason Tedor <[email protected]>
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v7.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.