You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/02/24 09:03:46 UTC
[GitHub] [incubator-doris] WingsGo opened a new pull request #2980:
[Conf]Make default_storage_medium configurable
WingsGo opened a new pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980
For #2954
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r392296461
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -170,7 +170,7 @@ public String toString() {
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
buff.append(entry.getKey()).append("is HDD: ");;
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r387533562
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -127,7 +127,7 @@ public void write(DataOutput out) throws IOException {
out.writeInt(idToDataProperty.size());
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
out.writeLong(entry.getKey());
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
I am confused why we should add a judge whether the DataProperty is default, can we just write `entry.getValue().write(out);` and read as `DataProperty.read(in)`. I think the idToDataProperty's values can't be null?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r392255041
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -127,7 +127,7 @@ public void write(DataOutput out) throws IOException {
out.writeInt(idToDataProperty.size());
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
out.writeLong(entry.getKey());
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
You are righ, it is a little bit confused..
You can change it as you said.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on issue #2980: [Conf]Make
default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman commented on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-591737881
So you mean, when all the disks of a cluster are replaced with SSD, we set this parameter to SSD?
Then if only a part of the disks in the cluster are replaced with SSDs, do we need to change this parameter to SSDs? If it is changed, then all new data will fall on the SSD?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r388068022
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -127,7 +127,7 @@ public void write(DataOutput out) throws IOException {
out.writeInt(idToDataProperty.size());
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
out.writeLong(entry.getKey());
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
If we need to compatible with old version, I can chnage it to `entry.getValue() == new DataProperty(TStorageMedium.HDD)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r392296522
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -127,7 +127,7 @@ public void write(DataOutput out) throws IOException {
out.writeInt(idToDataProperty.size());
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
out.writeLong(entry.getKey());
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r392256358
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -170,7 +170,7 @@ public String toString() {
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
buff.append(entry.getKey()).append("is HDD: ");;
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
Change the log print `is HDD: `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman merged pull request #2980: [Conf]Make
default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on a change in pull request
#2980: [Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#discussion_r387052630
##########
File path: fe/src/main/java/org/apache/doris/catalog/PartitionInfo.java
##########
@@ -127,7 +127,7 @@ public void write(DataOutput out) throws IOException {
out.writeInt(idToDataProperty.size());
for (Map.Entry<Long, DataProperty> entry : idToDataProperty.entrySet()) {
out.writeLong(entry.getKey());
- if (entry.getValue() == DataProperty.DEFAULT_HDD_DATA_PROPERTY) {
+ if (entry.getValue() == DataProperty.DEFAULT_DATA_PROPERTY) {
Review comment:
Here will be a problem.
For now, `DataProperty.DEFAULT_DATA_PROPERTY` is configuration, so here, it is all possible to write `true` or `false`. And when reading, `true` may point to HDD, may point to SSD, depends on the config.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on issue #2980: [Conf]Make
default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman commented on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-591737887
So you mean, when all the disks of a cluster are replaced with SSD, we set this parameter to SSD?
Then if only a part of the disks in the cluster are replaced with SSDs, do we need to change this parameter to SSDs? If it is changed, then all new data will fall on the SSD?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo edited a comment on issue #2980:
[Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo edited a comment on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-590913254
> I don't quite understand how this configuration solves the problem you mentioned in the #2954 ? For example, the current clusters are HDD, and now some machines need to be replaced with SSDs. How to use this configuration in this case?
Yes, this configuration can not change partitions already is HDD. But if cluster medium is changed from HDD to SSD, admin should change the history partitions storage medium. But for users, they may have no idea that the cluster storage_medium is changed or even they don't konw about they can specify the storage_medium.so in this case, they will create table as usual, this will cause all partitions's storage_medium after migration is default (HDD), but currently, the cluster have no machine who's medium is HDD, this will cause some problem about cluster balance.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] morningman commented on issue #2980: [Conf]Make
default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
morningman commented on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-590897385
I don't quite understand how this configuration solves the problem you mentioned in the #2954 ? For example, the current clusters are HDD, and now some machines need to be replaced with SSDs. How to use this configuration in this case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on issue #2980: [Conf]Make
default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo commented on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-591762513
In my opinion, this configuration gives admin a choice, if part of the disk are replaced with SSD, we don't need to change anything, the PR doesn't affect anything either. This PR is just an admin option, it will help admin control the system more conviently without change users's behavior. If the cluster is combine with HDD and SSD, admin should let users konwn what kind of table's medium should they choose.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo commented on issue #2980: [Conf]Make
default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo commented on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-590913254
> I don't quite understand how this configuration solves the problem you mentioned in the #2954 ? For example, the current clusters are HDD, and now some machines need to be replaced with SSDs. How to use this configuration in this case?
This configuration can not change partitions already is HDD，if cluster medium is changed from HDD to SSD, admin should change the history partitions storage medium. But for users, they may have no idea that the cluster storage_medium is changed or even they don't konw about they can specify the storage_medium.so in this case, they will create table as usual, this will cause all partitions's storage_medium after migration is default (HDD), but currently, the cluster have no machine who's medium is HDD, this will cause some problem about cluster balance.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] WingsGo edited a comment on issue #2980:
[Conf]Make default_storage_medium configurable
Posted by GitBox <gi...@apache.org>.
WingsGo edited a comment on issue #2980: [Conf]Make default_storage_medium configurable
URL: https://github.com/apache/incubator-doris/pull/2980#issuecomment-590913254
> I don't quite understand how this configuration solves the problem you mentioned in the #2954 ? For example, the current clusters are HDD, and now some machines need to be replaced with SSDs. How to use this configuration in this case?
Yes, this configuration can not change partitions already is HDD. But if cluster medium is changed from HDD to SSD, admin should change the history partitions storage medium. But for users, they may have no idea that the cluster storage_medium is changed or even they don't konw about they can specify the storage_medium.So in this case, they will create table as usual, this will cause all partitions's storage_medium after migration is default (HDD), but currently, the cluster have no machine who's medium is HDD, this will cause some problem about cluster balance.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org