Skip to content
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

Support setting properties for storage_root_path #2235

Merged
merged 17 commits into from
Nov 22, 2019

Conversation

vagetablechicken
Copy link
Contributor

ref #2195

We can specify the properties of storage_root_path by setting ':', seperate by ','
e.g.
storage_root_path = /home/disk1/palo,medium:ssd,capacity:50

@vagetablechicken
Copy link
Contributor Author

property medium has a higher priority, so I decide to set storage_medium in c'tor of DataDir, change
'_init_extension_and_capacity()' to '_init_capacity()'.
https://github.com/apache/incubator-doris/blob/14769b0beb7671626da011611d2c73ffe841b831/be/src/olap/data_dir.h#L132

But in meta_tool,
https://github.com/apache/incubator-doris/blob/14769b0beb7671626da011611d2c73ffe841b831/be/src/tools/meta_tool.cpp#L143
var 'data_dir' will not be set the _storage_medium, cause I remove the _init_extension part.

I don't want to redefine TStorageMedium enum, e.g. {UNSET, HDD,SSD}.
How about

    DataDir(const std::string& path,
            int64_t capacity_bytes = -1,
            std::string medium_property = "",
            TabletManager* tablet_manager = nullptr,
            TxnManager* txn_manager = nullptr);

Thus,

  1. medium_property != "", set _storage_medium
  2. medium_property == "", use extension

@imay
Copy link
Contributor

imay commented Nov 19, 2019

@vagetablechicken

  1. I think meta tool should not care about the medium of data path.
  2. Even if meta tool care about medium, it is better to make medium and capacity parse in one place. You can make config parse a function, which can be called by both options and meta dir.

Copy link
Contributor

@imay imay left a comment

Choose a reason for hiding this comment

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

Good Job
LGTM

@imay imay merged commit fda4665 into apache:master Nov 22, 2019
@vagetablechicken vagetablechicken deleted the medium branch November 22, 2019 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants