Move to one data.path per shard #10461

s1monw · 2015-04-07T14:57:42Z

This commit moves away from using stripe RAID-0 simumlation across multiple
data paths towards using a single path per shard. Multiple data paths are still
supported but shards and it's data is not striped across multiple paths / disks.
This will for instance prevent to loose all shards if a single disk is corrupted.

Indices that are using this features already will automatically upgraded to a single
datapath based on a simple diskspace based heuristic. In general there must be enough
diskspace to move a single shard at any time otherwise the upgrade will fail.

Closes #9498

s1monw · 2015-04-07T14:58:08Z

FYI this is a first cut at this feature so it's not perfect but it mirrors the direction pretty well..

rjernst · 2015-04-07T19:17:52Z

src/main/java/org/elasticsearch/common/util/MultiDataPathUpgrader.java

+import java.util.ArrayList;
+import java.util.List;
+
+/**


remove empty javadoc?

mikemccand · 2015-04-15T15:08:26Z

src/main/java/org/elasticsearch/common/util/MultiDataPathUpgrader.java

+                if (Files.exists(path.resolve(MetaDataStateFormat.STATE_DIR_NAME))) {
+                    numPathsExist++;
+                }
+                if (numPathsExist > 1) {


This if can be moved inside the one above it.

mikemccand · 2015-04-15T15:41:55Z

If ES crashes/is killed while a shard is being upgraded that shard is now corrupt right? Because we have moved some but not all files? Maybe in the upgrade release notes we strongly suggest taking snapshot before (maybe we do this already)?

mikemccand · 2015-04-15T15:47:50Z

src/main/java/org/elasticsearch/env/PathSelector.java

+                return dataPaths[i];
+            }
+        }
+        Path maxUseablePath = dataPaths[0];


extra 'e' here too

s1monw · 2015-04-15T15:56:22Z

If ES crashes/is killed while a shard is being upgraded that shard is now corrupt right? Because we have moved some but not all files? Maybe in the upgrade release notes we strongly suggest taking snapshot before (maybe we do this already)?

that is a great question, the answer is no IMO. Today if you have multiple datapaths and distributor directory you can stop upgradeing in the middle and still being able to open it with 1.x and you distributor dir. The distiributor doesn't care where the files are and we don't delete before we have successfully moved them so I think we are ok here?

mikemccand · 2015-04-15T15:58:25Z

src/main/java/org/elasticsearch/gateway/DanglingIndicesState.java

@@ -58,7 +57,7 @@ public DanglingIndicesState(Settings settings, NodeEnvironment nodeEnv, MetaStat
     * Process dangling indices based on the provided meta data, handling cleanup, finding
     * new dangling indices, and allocating outstanding ones.
     */
-    public void processDanglingIndices(MetaData metaData) {
+    public void processDanglingIndices(MetaData metaData){


Lost a space.

mikemccand · 2015-04-15T16:19:13Z

I love this change, I love all the stuff that's removed! I left some minor comments...

mikemccand · 2015-04-15T16:22:16Z

that is a great question, the answer is no IMO.

Oh I see, because on restart, the distributor will just look around and find where the file resides and just open it there? Good!

Hmm but what about the non-atomic move case? Does Files.move behave well if JVM crashes while it's running? E.g. copy to a temp file on the dest file store and then do an atomic rename in the end?

s1monw · 2015-04-16T09:53:00Z

Hmm but what about the non-atomic move case? Does Files.move behave well if JVM crashes while it's running? E.g. copy to a temp file on the dest file store and then do an atomic rename in the end?

yeah but when do you remove the source file, there is always a window here I guess?

s1monw · 2015-04-16T10:00:16Z

@mikemccand I pushed changes according to your comments, and merged with master

bleskes · 2015-04-17T10:28:29Z

src/main/java/org/elasticsearch/index/shard/ShardPath.java

+    }
+
+    // nocommit - we need something more extensible but this does the job for now...
+    public static ShardPath findShardPath(NodeEnvironment env, ShardId shardId, @IndexSettings Settings indexSettings) throws IOException {


can we name this selectNewPathForShard? , to contrast it from loadShardPath (findShardPath sounds to me like go and find an existing shard path).

bleskes · 2015-04-17T10:46:51Z

I this is good. Left little concerns/comments/suggestions here and there.

s1monw · 2015-04-20T08:49:06Z

@bleskes I pushed another commit adressing your comments. @mikemccand I moved to a more pessimistic model give your comments too by using atomic move etc. can you take another look

s1monw · 2015-04-20T09:59:51Z

@mikemccand pushed a new commit

mikemccand · 2015-04-20T10:01:21Z

LGTM, thanks @s1monw!

bleskes · 2015-04-20T11:04:00Z

src/main/java/org/elasticsearch/common/util/MultiDataPathUpgrader.java

+        logger.info("{} upgrading multi data dir to {}", shard, targetPath.getDataPath());
+        final ShardStateMetaData loaded = ShardStateMetaData.FORMAT.loadLatestState(logger, paths);
+        if (loaded == null) {
+            throw new IllegalStateException(shard + " no shard state found in any of: [" + Arrays.toString(paths) + "] please check and remove them if possible");


Arrays.toString(paths) already adds [] , no need to add them

bleskes · 2015-04-20T11:10:44Z

Thx @s1monw looks great. I left some very minor comment and replied to your question about #10461 (comment)

s1monw · 2015-04-20T13:18:52Z

@bleskes pushed one more commit - would you mind taking a look

bleskes · 2015-04-20T13:24:27Z

src/test/java/org/elasticsearch/index/shard/ShardPathTests.java

+            Path[] paths = env.availableShardPaths(shardId);
+            Path path = randomFrom(paths);
+            assumeTrue(paths.length > 1);
+            int id = randomIntBetween(1, 10);


we should remove path from paths. otherwise we write only one shard state no?

nevermind. Misread the test. It actually writes a shard copy to all paths.

I removed the second state creation

bleskes · 2015-04-20T13:49:29Z

LGTM. Left one comment we agreed to solve on a follow up issue.

s1monw · 2015-04-20T13:56:11Z

@bleskes I opened #10677 for this

This commit moves away from using stripe RAID-0 simumlation across multiple data paths towards using a single path per shard. Multiple data paths are still supported but shards and it's data is not striped across multiple paths / disks. This will for instance prevent to loose all shards if a single disk is corrupted. Indices that are using this features already will automatically upgraded to a single datapath based on a simple diskspace based heuristic. In general there must be enough diskspace to move a single shard at any time otherwise the upgrade will fail. Closes elastic#9498

s1monw added >enhancement v2.0.0-beta1 blocker :Distributed/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. labels Apr 7, 2015

s1monw assigned kimchy Apr 7, 2015

dakrone changed the title ~~[STORE] Move to on data.path per shard~~ [STORE] Move to one data.path per shard Apr 7, 2015

rjernst reviewed Apr 7, 2015
View reviewed changes

s1monw force-pushed the fix_multiple_data_path branch from 7385f37 to c82b8e3 Compare April 13, 2015 10:44

mikemccand reviewed Apr 15, 2015
View reviewed changes

s1monw unassigned kimchy Apr 16, 2015

bleskes reviewed Apr 17, 2015
View reviewed changes

bleskes reviewed Apr 20, 2015
View reviewed changes

s1monw mentioned this pull request Apr 20, 2015

Ensure we can recover cleanly into a leftover shard directory from a different index #10677

Closed

s1monw force-pushed the fix_multiple_data_path branch 2 times, most recently from 91488c5 to 3c76333 Compare April 20, 2015 14:25

s1monw force-pushed the fix_multiple_data_path branch from 6b393be to 5730c06 Compare April 20, 2015 15:19

s1monw merged commit 5730c06 into elastic:master Apr 20, 2015

s1monw deleted the fix_multiple_data_path branch April 20, 2015 19:02

mikemccand mentioned this pull request May 12, 2015

Shard allocation should work harder to balance new shards across multiple path.data paths #11122

Closed

clintongormley changed the title ~~[STORE] Move to one data.path per shard~~ Move to one data.path per shard Jun 7, 2015

clintongormley added :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Distributed/Store Issues around managing unopened Lucene indices. If it touches Store.java, this is a likely label. labels Feb 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move to one data.path per shard #10461

Move to one data.path per shard #10461

s1monw commented Apr 7, 2015

s1monw commented Apr 7, 2015

rjernst Apr 7, 2015

mikemccand Apr 15, 2015

mikemccand commented Apr 15, 2015

mikemccand Apr 15, 2015

s1monw commented Apr 15, 2015

mikemccand Apr 15, 2015

mikemccand commented Apr 15, 2015

mikemccand commented Apr 15, 2015

s1monw commented Apr 16, 2015

s1monw commented Apr 16, 2015

bleskes Apr 17, 2015

bleskes commented Apr 17, 2015

s1monw commented Apr 20, 2015

s1monw commented Apr 20, 2015

mikemccand commented Apr 20, 2015

bleskes Apr 20, 2015

bleskes commented Apr 20, 2015

s1monw commented Apr 20, 2015

bleskes Apr 20, 2015

s1monw Apr 20, 2015

bleskes Apr 20, 2015

s1monw Apr 20, 2015

bleskes commented Apr 20, 2015

s1monw commented Apr 20, 2015

Move to one data.path per shard #10461

Move to one data.path per shard #10461

Conversation

s1monw commented Apr 7, 2015

s1monw commented Apr 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikemccand commented Apr 15, 2015

Choose a reason for hiding this comment

s1monw commented Apr 15, 2015

Choose a reason for hiding this comment

mikemccand commented Apr 15, 2015

mikemccand commented Apr 15, 2015

s1monw commented Apr 16, 2015

s1monw commented Apr 16, 2015

Choose a reason for hiding this comment

bleskes commented Apr 17, 2015

s1monw commented Apr 20, 2015

s1monw commented Apr 20, 2015

mikemccand commented Apr 20, 2015

Choose a reason for hiding this comment

bleskes commented Apr 20, 2015

s1monw commented Apr 20, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bleskes commented Apr 20, 2015

s1monw commented Apr 20, 2015