You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by arina-ielchiieva <gi...@git.apache.org> on 2016/04/08 19:02:55 UTC
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
GitHub user arina-ielchiieva opened a pull request:
https://github.com/apache/drill/pull/474
DRILL-4596: Drill should do version check among drillbits
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/arina-ielchiieva/drill DRILL-4596
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/474.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #474
----
commit 76e857c29becee22daf116dd58a36fe34a920595
Author: Arina Ielchiieva <ar...@gmail.com>
Date: 2016-04-08T14:22:47Z
DRILL-4596: Drill should do version check among drillbits
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by JohnOmernik <gi...@git.apache.org>.
Github user JohnOmernik commented on a diff in the pull request:
https://github.com/apache/drill/pull/474#discussion_r59064857
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +209,28 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * Disallow registering drillbit when:
+ * 1. version is unknown;
+ * 2. drillbit with different version has been already registered.
--- End diff --
Agreed. In addition to potential soft enforcement, I think we should consider the differences of a version defined in a pom.xml, the version of the running drillbit, and the version of a drill cluster. By using a setting in drill-override to specify what the drill cluster version is, we don't get accident "first bit to register is the wrong version" situations. Are there situations where we might want to allow a specific Drill version + all minor versions at that level, or Drill version + 1 Major version for rolling upgrades?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-209081311
In general, I'm against version number checking. We did that in the code early on but we should be moving towards a capabilities flag approach.
Also agree with Paul in his mention of DRILL-4286, don't think worrying about rolling upgrade makes sense until we resolve the issues around decommissioning.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #474: DRILL-4596: Drill should do version check among dri...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva closed the pull request at:
https://github.com/apache/drill/pull/474
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by yufeldman <gi...@git.apache.org>.
Github user yufeldman commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208554692
Not sure how that comment contradicts with what I said. Adding new feature in the next version should not break previous version.
> On Apr 11, 2016, at 12:41 PM, arina-ielchiieva <no...@github.com> wrote:
>
> I thought it's hard to maintain backward compatibility. Quoting Parth comment from Jira:
> "Drill needs to be able to run queries that depend only on features that are in the oldest version of Drill in the cluster. Say for example, most drillbits are at version N and some are at version N+1. You added support for a new type foo in version N+1. Any queries not using or depending on type foo should run successfully. Queries depending on foo can only run successfully after all the drillbits are upgraded to N+1."
>
> —
> You are receiving this because you commented.
> Reply to this email directly or view it on GitHub
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208428822
Updated PR.
New implementation approach:
1. we allow to register any drillbit (disregarding the version).
2. instead of getting available endpoints from coordinator, we'll be receiving compatible endpoints. Version compatibility will be defined using main application version (major.minor (+ 1). For example, drillbit with version 1.8 can work with 1.8 and 1.9. Drillbit with version 1.9 can work with 1.9 and 2.0 and so on. Even during rolling upgrades, compatible endpoints will be able to work together.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by yufeldman <gi...@git.apache.org>.
Github user yufeldman commented on a diff in the pull request:
https://github.com/apache/drill/pull/474#discussion_r59063793
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +209,28 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * Disallow registering drillbit when:
+ * 1. version is unknown;
+ * 2. drillbit with different version has been already registered.
--- End diff --
What if you first DrillBit registered had "bad" version? Somebody would have to bring it down, so others can start?
Should it be (at least) option to have soft enforcement, rather than just hard enforcement.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by JohnOmernik <gi...@git.apache.org>.
Github user JohnOmernik commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-207520784
So how is the version actually set? Is it just the bit to register with Drill? Is there a config setting that we can set in drill-override that sets the cluster version? Where is the "master" version stored, and how do we move away from that?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208943460
Hi All. Version compatibility is a complex issue. Do we have a design document that explains our goals and policy? Is the goal to allow rolling updates of clients? (Drill server at, say 1.8, rolling upgrade of client from 1.7 to 1.8, old clients work)? Or, is it to allow rolling upgrades of servers? Both?
MapR customers receive even releases: 1.4, 1.6, 1.8. Would the +/-1 policy benefit them?
As I understand it, each Drill bit is to work with another of +/-1 version. But, what I bring up Drill 1.6, Drill 1.5 and Drill 1.4. The 1.5 is happy to work with both 1.6 and 1.4. But, the 1.6 and 1.4 versions will fail only when they communicate with one another. When will this communication occur? At startup? Or, only later when, say, 1.6 tries to send a query to 1.4?
Does this mean that Drillbits should advertise their version in ZooKeeper so that we fail fast and can provide a clear error message?
Dremio proposes a new 2.0 release that breaks compatibility. Will Drill 1.9 (say) be compatible with the incompatile Drill 2.0? Should it be?
As others have said, we need to consider wire protocol and semantics. The usual solution is protocol negotiation. If a 1.6 client connects to a 1.7 server, they agree to "speak" 1.6. If a 1.7 client connects to a 1.6 server, they also agree to "speak" 1.6. Such as solution has impact on our messaging layer. It increases testing requirements.
Drill-on-YARN will provide another way to do server upgrades (ramp up a new cluster while ramping down an old one.) Otherwise, YARN will need some way to run the same cluster, replacing version X drillbits with version X+1 (while still running the version X Application Master).
Are all these issues spelled out in a design doc?
IMHO: let's not try to bug fix our way to success here; let's step back and work out a complete design.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208521519
I thought it's hard to maintain backward compatibility. Quoting Parth comment from Jira:
"Drill needs to be able to run queries that depend only on features that are in the oldest version of Drill in the cluster. Say for example, most drillbits are at version N and some are at version N+1. You added support for a new type foo in version N+1. Any queries not using or depending on type foo should run successfully. Queries depending on foo can only run successfully after all the drillbits are upgraded to N+1."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-207529907
Drill version is defined from build manifest file [1].
There is no config setting to override it. Though if you want to change version in "master", you'll have to update version property in pom.xml files. (<version>1.7.0-SNAPSHOT</version>)
[1] https://github.com/apache/drill/blob/master/common/src/main/java/org/apache/drill/common/util/DrillVersionInfo.java
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by yufeldman <gi...@git.apache.org>.
Github user yufeldman commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208434359
Should it be +-1, not just +1 ? Otherwise in your scheme 1.9 can work with
2.0, but can not with 1.8, while 1.8 can work with 1.9.
On Mon, Apr 11, 2016 at 9:08 AM, arina-ielchiieva <no...@github.com>
wrote:
> Updated PR.
> New implementation approach:
> 1. we allow to register any drillbit (disregarding the version).
> 2. instead of getting available endpoints from coordinator, we'll be
> receiving compatible endpoints. Version compatibility will be defined using
> main application version (major.minor (+ 1). For example, drillbit with
> version 1.8 can work with 1.8 and 1.9. Drillbit with version 1.9 can work
> with 1.9 and 2.0 and so on. Even during rolling upgrades, compatible
> endpoints will be able to work together.
>
> —
> You are receiving this because you commented.
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/drill/pull/474#issuecomment-208428822>
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/474#discussion_r59077998
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +209,28 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * Disallow registering drillbit when:
+ * 1. version is unknown;
+ * 2. drillbit with different version has been already registered.
+ */
+ private void checkVersion(DrillbitEndpoint endpoint) throws DrillbitStartupException {
+ String currentVersion = endpoint.getVersion();
+ if (DrillVersionInfo.UNKNOWN_VERSION.equals(currentVersion)) {
+ throw new DrillbitStartupException("Drillbit version is unknown.");
+ }
+
+ for (DrillbitEndpoint registeredEnpoint : coord.getAvailableEndpoints()) {
+ if (!currentVersion.equals(registeredEnpoint.getVersion())) {
--- End diff --
Agree. Probably it's better to compare each module major versions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by paul-rogers <gi...@git.apache.org>.
Github user paul-rogers commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208959253
Some other design issues. The idea of a rollling upgrade presupposes that we can shut down a Drillbit, bring up a new one, and the cluster keeps running. But, today, bringing down a Drillbit causes all in-flight queries on that node to fail. There is no way to mark a node as "quiescent" (up, but not accepting new work.) So, a rolling upgrade today would entail a long series of query failures as we replace each of, say, 20 or 50 nodes. So, in fact, it is less disruptive to take the cluster down, push an upgrade, and bring it back up. (See DRILL-4286.)
Back on testing: testing is essential. A feature that allow +/-1 feature compatibility is not helpful unless someone (other than the user) can certify that it works. If the user gets to do the checking, then it is not very helpful: safer just to do a full upgrade.
To emphasize an earlier point: there are two distinct issues. One is a managed cluster upgrade (the admin can do it with the help of a management tool.) The other are the many Drill clients spread across desktops: that is a classic desktop software upgrade. Some might be on planes, others locked in desks while someone is on vacation. Let's think about how to upgrade JDBC drivers and the like given this reality.
Is the compatiblity policy number or time based? As an admin, can I expect to have a three-month window for upgrades? Or, will it sometimes be one month, others four months, depending on who changes what? Should we have a time-based policy?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by sudheeshkatkam <gi...@git.apache.org>.
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/474#discussion_r59065316
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +209,28 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * Disallow registering drillbit when:
+ * 1. version is unknown;
+ * 2. drillbit with different version has been already registered.
+ */
+ private void checkVersion(DrillbitEndpoint endpoint) throws DrillbitStartupException {
+ String currentVersion = endpoint.getVersion();
+ if (DrillVersionInfo.UNKNOWN_VERSION.equals(currentVersion)) {
+ throw new DrillbitStartupException("Drillbit version is unknown.");
+ }
+
+ for (DrillbitEndpoint registeredEnpoint : coord.getAvailableEndpoints()) {
+ if (!currentVersion.equals(registeredEnpoint.getVersion())) {
--- End diff --
How does this check work with regards to [protobuf backwards compatibility](https://developers.google.com/protocol-buffers/docs/javatutorial#extending-a-protocol-buffer)? Endpoints currently do not have a version. So if two bits (A and B), one with and one without this change, are started, the two start order cases (A after B, and B after A) should pass.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on the pull request:
https://github.com/apache/drill/pull/474#issuecomment-208557847
If we add new type to version 1.7, it can't work with version 1.6 where this type is absent, that's why I am adding support only for +1 version. At this point in my implementation, 1.6 can work with 1.6 and 1.7. 1.7 can work with 1.7 and 1.8.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by yufeldman <gi...@git.apache.org>.
Github user yufeldman commented on a diff in the pull request:
https://github.com/apache/drill/pull/474#discussion_r59297387
--- Diff: common/src/main/java/org/apache/drill/common/util/DrillVersionInfo.java ---
@@ -49,10 +53,52 @@ public static String getVersion() {
}
}
} catch (IOException except) {
- appVersion = "Unknown";
+ appVersion = UNKNOWN_VERSION;
}
return appVersion;
}
+ /**
+ * Compare two Drill versions disregarding build number and comparing only major and minor versions.
+ * Versions are considered to be compatible:
+ * 1. if current version is the same as version to compare.
+ * 2. if current version minor version + 1 is the same as version to compare.
+ */
+ public static boolean isVersionsCompatible(String currentVersion, String versionToCompare) {
+ if (currentVersion != null && currentVersion.equals(versionToCompare)) {
+ return true;
+ }
+
+ BigDecimal currentVersionDecimal = getVersionAsDecimal(currentVersion);
+ BigDecimal versionToCompareDecimal = getVersionAsDecimal(versionToCompare);
+
+ if (currentVersionDecimal != null && versionToCompareDecimal != null) {
--- End diff --
So for Drillbit with version 1.6 Drillbit with version 1.7 will be included, while for a Drillbit with version 1.7 Drillbit with version 1.6 will not be included? Does it mean that at any particular time a particular Drillbit will have different view on the cluster from some other Drillbit?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request: DRILL-4596: Drill should do version check amon...
Posted by arina-ielchiieva <gi...@git.apache.org>.
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/474#discussion_r59332891
--- Diff: common/src/main/java/org/apache/drill/common/util/DrillVersionInfo.java ---
@@ -49,10 +53,52 @@ public static String getVersion() {
}
}
} catch (IOException except) {
- appVersion = "Unknown";
+ appVersion = UNKNOWN_VERSION;
}
return appVersion;
}
+ /**
+ * Compare two Drill versions disregarding build number and comparing only major and minor versions.
+ * Versions are considered to be compatible:
+ * 1. if current version is the same as version to compare.
+ * 2. if current version minor version + 1 is the same as version to compare.
+ */
+ public static boolean isVersionsCompatible(String currentVersion, String versionToCompare) {
+ if (currentVersion != null && currentVersion.equals(versionToCompare)) {
+ return true;
+ }
+
+ BigDecimal currentVersionDecimal = getVersionAsDecimal(currentVersion);
+ BigDecimal versionToCompareDecimal = getVersionAsDecimal(versionToCompare);
+
+ if (currentVersionDecimal != null && versionToCompareDecimal != null) {
--- End diff --
I guess, yes. Ideally all drillbits in cluster will be with the same version, only during rolling upgrades this situation may occur.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---