Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Use indexes subdirectory for custom index system path #369

Open
sezruby wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Use indexes subdirectory for custom index system path #369

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d28e14f
Use indexes subdirectory for custom index system path
sezruby Feb 26, 2021
2a47dc7
Merge remote-tracking branch 'upstream/master' into path
sezruby Mar 5, 2021
fec013a
Add INDEX_DIR_NAME config
sezruby Mar 5, 2021
4423b3b
Merge remote-tracking branch 'upstream/master' into path
sezruby Apr 12, 2021
4537281
Change systemPath => indexLocationDir
sezruby Apr 12, 2021
a2833ef
Merge branch 'master' into path
sezruby Apr 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/main/scala/com/microsoft/hyperspace/index/PathResolver.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,8 @@ private[hyperspace] class PathResolver(conf: SQLConf, hadoopConf: Configuration)
* @return Hyperspace index system path.
*/
def systemPath: Path = {

@clee704 clee704 Apr 9, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm a little bit confused by now about what a system path is in Hyperspace, because the value of INDEX_SYSTEM_PATH and the return value of this function can be different, yet both are being called "system paths". Which one is the "system path"? Shouldn't we name either one differently? Maybe we can 1) improve the docstring, 2) change the config name from INDEX_SYSTEM_PATH to something like INDEX_SYSTEM_ROOT, and/or 3) use a more self-explanatory name than system path.

@sezruby sezruby Apr 9, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I think it's better to change the function name rather than the config string. So my suggestion is 1) keep "system path" as it is (w/o subdir) 2) give another name to "system path"/subdir path.

How about indexLocationPath?

cc @imback82

val defaultIndexesPath =
new Path(conf.getConfString("spark.sql.warehouse.dir"), "indexes")
new Path(conf.getConfString(IndexConstants.INDEX_SYSTEM_PATH, defaultIndexesPath.toString))
val defaultIndexesPath = conf.getConfString("spark.sql.warehouse.dir")
new Path(conf.getConfString(IndexConstants.INDEX_SYSTEM_PATH, defaultIndexesPath), "indexes")
Comment thread
sezruby marked this conversation as resolved.
Outdated
}
}

Expand Down