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

[#2886] improvement(ci): optimize Docker container to suite CI framework #2887

Merged
merged 2 commits into from
Apr 13, 2024

Conversation

zhoukangcn
Copy link
Contributor

@zhoukangcn zhoukangcn commented Apr 11, 2024

What changes were proposed in this pull request?

  • remove log from container stdout
  • improvement: add config to avoid storage_medium_check

Why are the changes needed?

Fix: #2886

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

CI

@@ -37,6 +36,7 @@ PRIORITY_NETWORKS=$(echo "${CONTAINER_IP}" | awk -F '.' '{print$1"."$2"."$3".0/2
echo "add priority_networks = ${PRIORITY_NETWORKS} to fe.conf & be.conf"
echo "priority_networks = ${PRIORITY_NETWORKS}" >> ${DORIS_FE_HOME}/conf/fe.conf
echo "priority_networks = ${PRIORITY_NETWORKS}" >> ${DORIS_BE_HOME}/conf/be.conf
echo "disable_storage_medium_check = true" >> ${DORIS_FE_HOME}/conf/fe.conf
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this configuration prevent the check for left storage when starting?

Copy link
Contributor Author

@zhoukangcn zhoukangcn Apr 11, 2024

Choose a reason for hiding this comment

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

This config is not to check storage space. This config si check storage medium to store cold data.

If disable_storage_medium_check is true, ReportHandler would not check tablet's storage medium and disable storage cool down function, the default value is false. You can set the value true when you don't care what the storage medium of the tablet is.

When we have SSD and HDD on same machine, this check is helpful.

When we use in CI, this config may cause BE refuse request, and finally make create table failed probabilistic (when the container has just been started)

I am still checking it in CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After test, I think we don't need to change config disable_storage_medium_check.

BUT we need to set report_disk_state_interval_seconds to 10 seconds, this config allows the BE to report storage information faster after startup. This can prevent DorisTableIT from failing.

yuqi1129
yuqi1129 previously approved these changes Apr 11, 2024
Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

LGTM

@zhoukangcn zhoukangcn closed this Apr 11, 2024
@zhoukangcn zhoukangcn reopened this Apr 11, 2024
@zhoukangcn zhoukangcn changed the title [#2886] improvement(ci): opimize Docker container to suite CI framework [WIP][#2886] improvement(ci): opimize Docker container to suite CI framework Apr 11, 2024
@zhoukangcn zhoukangcn changed the title [WIP][#2886] improvement(ci): opimize Docker container to suite CI framework [WIP][#2886] improvement(ci): optimize Docker container to suite CI framework Apr 11, 2024
@zhoukangcn zhoukangcn changed the title [WIP][#2886] improvement(ci): optimize Docker container to suite CI framework [#2886] improvement(ci): optimize Docker container to suite CI framework Apr 11, 2024
@yuqi1129
Copy link
Contributor

@zhoukangcn
Could you change the image version to 0.1.3 by the way, I have verified it.

@zhoukangcn
Copy link
Contributor Author

@zhoukangcn
Could you change the image version to 0.1.3 by the way, I have verified it.

OK,I can do it later

@zhoukangcn
Copy link
Contributor Author

@yuqi1129 done. Please take a look when you have time.

@zhoukangcn
Copy link
Contributor Author

@yuqi1129 please review PR #2922

Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

LGTM

@yuqi1129 yuqi1129 merged commit 282d082 into apache:main Apr 13, 2024
22 checks passed
@zhoukangcn zhoukangcn deleted the doris_docker_minor branch April 13, 2024 11:50
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.

[Subtask] Optimize the Doris container to suit container log upload
2 participants