-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-38909][CORE][YARN] Encapsulate LevelDB used to store remote/external shuffle state as DB
#36200
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
[SPARK-38909][CORE][YARN] Encapsulate LevelDB used to store remote/external shuffle state as DB
#36200
Changes from all commits
181a597
1e5654a
a4ae04c
29a7a1e
eacc159
ea21178
f57578e
ad5b74f
4d5739b
610e04b
c66c483
6e0a6f7
9ba181b
c80024b
1c3f09d
f03ea29
ecba36a
23753f5
4630651
712f99d
01a6da1
8e11695
9c553a8
797f4ca
d5bcce9
c6f6fa0
768f207
217df56
9b20939
515204d
28ee973
298c737
a851c88
1832b06
e147c77
004c4c9
a56f3c8
9df4fe5
7e1e562
0f7b348
c56a8f8
78d163e
bee0737
944c290
cf7658c
e3e37a7
d5a53a5
f8cb374
35bcced
2dc68c3
ee1f283
5e856f6
9590e97
b299c55
94a2831
d4ce629
9889de3
abb0a58
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.network.shuffledb; | ||
|
|
||
| import java.io.Closeable; | ||
|
|
||
| /** | ||
| * The local KV storage used to persist the shuffle state, | ||
| * the implementations may include LevelDB, RocksDB, etc. | ||
| */ | ||
| public interface DB extends Closeable { | ||
| /** | ||
| * Set the DB entry for "key" to "value". | ||
| */ | ||
| void put(byte[] key, byte[] value); | ||
|
|
||
| /** | ||
| * Get which returns a new byte array storing the value associated | ||
| * with the specified input key if any. | ||
| */ | ||
| byte[] get(byte[] key); | ||
|
|
||
| /** | ||
| * Delete the DB entry (if any) for "key". | ||
| */ | ||
| void delete(byte[] key); | ||
|
|
||
| /** | ||
| * Return an iterator over the contents of the DB. | ||
| */ | ||
| DBIterator iterator(); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.network.shuffledb; | ||
|
|
||
| import java.util.Locale; | ||
|
|
||
| /** | ||
| * The enum `DBBackend` use to specify a disk-based store used in shuffle service local db. | ||
| * Only LEVELDB is supported now. | ||
| */ | ||
| public enum DBBackend { | ||
| LEVELDB(".ldb"); | ||
tgravescs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| private final String fileSuffix; | ||
|
|
||
| DBBackend(String fileSuffix) { | ||
| this.fileSuffix = fileSuffix; | ||
| } | ||
|
|
||
| public String fileName(String prefix) { | ||
| return prefix + fileSuffix; | ||
| } | ||
|
|
||
| public static DBBackend byName(String value) { | ||
| return DBBackend.valueOf(value.toUpperCase(Locale.ROOT)); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.network.shuffledb; | ||
|
|
||
| import java.io.Closeable; | ||
| import java.util.Iterator; | ||
| import java.util.Map; | ||
|
|
||
| public interface DBIterator extends Iterator<Map.Entry<byte[], byte[]>>, Closeable { | ||
|
|
||
| /** | ||
| * Position at the first entry in the source whose `key` is at target. | ||
| */ | ||
| void seek(byte[] key); | ||
|
|
||
| default void remove() { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||
| /* | ||||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||||
| * contributor license agreements. See the NOTICE file distributed with | ||||
| * this work for additional information regarding copyright ownership. | ||||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||||
| * (the "License"); you may not use this file except in compliance with | ||||
| * the License. You may obtain a copy of the License at | ||||
| * | ||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||
| * | ||||
| * Unless required by applicable law or agreed to in writing, software | ||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||
| * See the License for the specific language governing permissions and | ||||
| * limitations under the License. | ||||
| */ | ||||
|
|
||||
| package org.apache.spark.network.shuffledb; | ||||
|
|
||||
| import java.io.IOException; | ||||
|
|
||||
| public class LevelDB implements DB { | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if we can reuse the following.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to the idea of leveraging existing code in kvstore. Right now, this part of the code is only being used by HistoryServer. But I think it is reasonable to either extend or refactor to make it being used for DBs in Shuffle.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a little difficult, as @mridulm said:
|
||||
| private final org.iq80.leveldb.DB db; | ||||
|
|
||||
| public LevelDB(org.iq80.leveldb.DB db) { | ||||
| this.db = db; | ||||
| } | ||||
|
|
||||
| @Override | ||||
| public void put(byte[] key, byte[] value) { | ||||
| db.put(key, value); | ||||
| } | ||||
|
|
||||
| @Override | ||||
| public byte[] get(byte[] key) { | ||||
| return db.get(key); | ||||
| } | ||||
|
|
||||
| @Override | ||||
| public void delete(byte[] key) { | ||||
| db.delete(key); | ||||
| } | ||||
|
|
||||
| @Override | ||||
| public void close() throws IOException { | ||||
| db.close(); | ||||
| } | ||||
|
|
||||
| @Override | ||||
| public DBIterator iterator() { | ||||
| return new LevelDBIterator(db.iterator()); | ||||
| } | ||||
| } | ||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.network.shuffledb; | ||
|
|
||
| import com.google.common.base.Throwables; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Map; | ||
| import java.util.NoSuchElementException; | ||
|
|
||
| public class LevelDBIterator implements DBIterator { | ||
|
|
||
| private final org.iq80.leveldb.DBIterator it; | ||
|
|
||
| private boolean checkedNext; | ||
|
|
||
| private boolean closed; | ||
|
|
||
| private Map.Entry<byte[], byte[]> next; | ||
|
|
||
| public LevelDBIterator(org.iq80.leveldb.DBIterator it) { | ||
| this.it = it; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean hasNext() { | ||
| if (!checkedNext && !closed) { | ||
| next = loadNext(); | ||
| checkedNext = true; | ||
| } | ||
| if (!closed && next == null) { | ||
| try { | ||
| close(); | ||
| } catch (IOException ioe) { | ||
| throw Throwables.propagate(ioe); | ||
| } | ||
| } | ||
| return next != null; | ||
| } | ||
|
|
||
| @Override | ||
| public Map.Entry<byte[], byte[]> next() { | ||
| if (!hasNext()) { | ||
| throw new NoSuchElementException(); | ||
| } | ||
| checkedNext = false; | ||
| Map.Entry<byte[], byte[]> ret = next; | ||
| next = null; | ||
| return ret; | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| if (!closed) { | ||
| it.close(); | ||
| closed = true; | ||
| next = null; | ||
| } | ||
mridulm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| public void seek(byte[] key) { | ||
| it.seek(key); | ||
| } | ||
|
|
||
| @Override | ||
| public void remove() { | ||
| throw new UnsupportedOperationException(); | ||
| } | ||
|
|
||
| private Map.Entry<byte[], byte[]> loadNext() { | ||
| boolean hasNext = it.hasNext(); | ||
| if (!hasNext) { | ||
| return null; | ||
| } | ||
| return it.next(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.network.shuffledb; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| /** | ||
| * Used to identify the version of data stored in local shuffle state DB. | ||
| */ | ||
| public class StoreVersion { | ||
tgravescs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| public static final byte[] KEY = "StoreVersion".getBytes(StandardCharsets.UTF_8); | ||
|
|
||
| public final int major; | ||
| public final int minor; | ||
|
|
||
| @JsonCreator | ||
| public StoreVersion(@JsonProperty("major") int major, @JsonProperty("minor") int minor) { | ||
| this.major = major; | ||
| this.minor = minor; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
|
|
||
| StoreVersion that = (StoreVersion) o; | ||
|
|
||
| return major == that.major && minor == that.minor; | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| int result = major; | ||
| result = 31 * result + minor; | ||
| return result; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.spark.network.util; | ||
|
|
||
| import java.io.File; | ||
| import java.io.IOException; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.google.common.annotations.VisibleForTesting; | ||
|
|
||
| import org.apache.spark.network.shuffledb.DBBackend; | ||
| import org.apache.spark.network.shuffledb.LevelDB; | ||
| import org.apache.spark.network.shuffledb.DB; | ||
| import org.apache.spark.network.shuffledb.StoreVersion; | ||
|
|
||
| public class DBProvider { | ||
| public static DB initDB( | ||
| DBBackend dbBackend, | ||
| File dbFile, | ||
| StoreVersion version, | ||
| ObjectMapper mapper) throws IOException { | ||
| if (dbFile != null) { | ||
| // TODO: SPARK-38888, add rocksdb implementation. | ||
| switch (dbBackend) { | ||
| case LEVELDB: | ||
| org.iq80.leveldb.DB levelDB = LevelDBProvider.initLevelDB(dbFile, version, mapper); | ||
| return levelDB != null ? new LevelDB(levelDB) : null; | ||
| default: | ||
| throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of relying on extension, we should make it explicit - for example what is in SPARK-37680.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me think about it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc314e6 add a new config
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, the type of If re-parse the type of Do you have any suggestions for this? @mridulm
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 35bcced change to pass |
||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| public static DB initDB(DBBackend dbBackend, File file) throws IOException { | ||
| if (file != null) { | ||
| // TODO: SPARK-38888, add rocksdb implementation. | ||
| switch (dbBackend) { | ||
| case LEVELDB: return new LevelDB(LevelDBProvider.initLevelDB(file)); | ||
| default: | ||
| throw new IllegalArgumentException("Unsupported DBBackend: " + dbBackend); | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a developer API or user API? I didn't see the custom DB could be plugged in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
developer API. The implementation of RocksDB is under review: #37610,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So could you add annotation
@DeveloperApifor it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not support users plugging in a different DB instance - it comes from within spark code.
Supported DB's come from
DBBackendandDBProvider.initDBhas an explicit switch for the supported types.Wont be @DeveloperAPI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mridulm It's a public interface. I think it should either be a user API or developer API, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... should we use
@Privateaskvstore.KVStore?spark/common/kvstore/src/main/java/org/apache/spark/util/kvstore/KVStore.java
Lines 65 to 66 in 52cd037
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration is public @Ngone51, not the implementation itself.
To put it differently, there is no way for users to leverage any of this - unless they modify spark code.
I am fine with marking it as
@Privateif we need to make the intent clearer.It is the same for most submodules in common/* - except probably for common/kvstore, where Marcello marked the interfaces as @Private :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mridulm @LuciferYang Thanks.