Resiliency: Be more conservative if index.version.created is not set #8018
Conversation
|
LGTM, the only concern in have is that we do similar checks in other place is the code , searching for usage of |
|
What about throwing an exception if it is not set? Things would very likely go wrong anyway if it is not set? |
|
@jpountz we started setting it on indices as of 0.19. This is an index meta data so there is no upgrade path. Not sure what to do with indices that were created with 0.18 and long since upgraded. Officially we are still supposed to be able to read them? Maybe it should be part of the upgrade API that once we are guaranteed that all segments of an index are on the current lucene version, we can update this setting. Not sure what can of worms that will open though. We can consider dropping this with 2.0 but we should let people know and offer them a way out. |
I agree there should be only one way to get this maybe we can just use the Version utils everywhere?
I kind of agree - I wonder what would happen if we missed something here? I mean there is no real upgradepath if shit hits the fan?
that is actually not true - there is an upgrade path in |
Fair enough :)
I wonder how bad it is to run with the wrong version (i.e., too old in this case). Wouldn't that cause way more subtle exceptions? I see things around field data and analysis. If we are concerned about this maybe we should have a work around cluster level settings which assigns a version instead of throwing an exception. We can potentially only do it in 1.x. |
|
I agree though we should really throw an exception here :) |
In case doc values are enabled, this can be pretty bad as eg. we recently switched from binary doc values to sorted numerics for numeric fields and lucene would refuse to index a document that has a doc value type that is different from what is currently indexed.
+1 on an exception. It would be much better than digging an issue for hours before noticing that the cause was that the version was not set. |
s1monw
force-pushed
the
created_version
branch
2 times, most recently
from
3d556fe
to
e531274
Compare
|
ok guys I changed this to barf if the setting is not present and I am surprised how many failures I got.... Yet I think this is the safest option to be honest! |
|
LGTM I agree this is safer, I like this option better! |
|
+1 |
Today we use the current version which might enable features that are not supported. We should throw an exception if this setting is not present for any index. Closes elastic#8018
s1monw
force-pushed
the
created_version
branch
from
e531274
to
ac4b39b
Compare
Today we use the current version which might enable features that are not supported. We should throw an exception if this setting is not present for any index. Closes elastic#8018
Today we use the current version which might enable features that are not supported. We should throw an exception if this setting is not present for any index. Closes elastic#8018
|
This patch breaks some plugins because of a signature change for It means that we will need to release analysis plugins for elasticsearch 1.4.0 in addition to 1.4.0.Beta1. |
|
wait, the |
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #87. (cherry picked from commit b3b0d34)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #87.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #87. (cherry picked from commit b3b0d34)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #33.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #33. (cherry picked from commit cb869cd)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #33. (cherry picked from commit cb869cd)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #33.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #33.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #33.
|
ok it seems like not a breaking change for the plugins |
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #47. (cherry picked from commit 3a90982)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #47. (cherry picked from commit 3a90982)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #47.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #31. (cherry picked from commit 3eeb827)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #31.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #31. (cherry picked from commit 3eeb827)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #41. (cherry picked from commit 75b800f)
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #41.
Due to this [change](elastic/elasticsearch#8018), we need to fix our tests for elasticsearch 1.4.0 and above. Closes #41. (cherry picked from commit 75b800f)
clintongormley
changed the title
Today we use the current version which might enable features that are not supported. We should throw an exception if this setting is not present for any index. Closes elastic#8018
Today we use the current version which might enable features that are
not supported. We should use the minimal known version rather than
defaulting to the current.