RATIS-1677. Do not auto format RaftStorage in RECOVER.#718
RATIS-1677. Do not auto format RaftStorage in RECOVER.#718szetszwo merged 3 commits intoapache:masterfrom
Conversation
lokeshj1703
left a comment
lokeshj1703
left a comment
There was a problem hiding this comment.
@szetszwo Thanks for working on this! The changes look good to me. I have a minor comment inline.
ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java
Outdated
Show resolved
Hide resolved
lokeshj1703
left a comment
lokeshj1703
left a comment
There was a problem hiding this comment.
@szetszwo Thanks for updating! The changes look good.
ratis-server/src/main/java/org/apache/ratis/server/storage/StorageImplUtils.java
Outdated
Show resolved
Hide resolved
lokeshj1703
left a comment
lokeshj1703
left a comment
There was a problem hiding this comment.
@szetszwo Thanks for updating the PR! The changes look good to me. +1.
|
@lokeshj1703 , thanks a lot for reviewing this! |
(cherry picked from commit ebb39e8)
(cherry picked from commit ebb39e8)
|
Hi @szetszwo, thank you for this fix.
So do we need revert this commit in 2.4.0 branch or make newAdd backwards compatible? cc @codings-dan @adoroszlai |
|
@captainzmc , thanks a lot for testing the rc1!
Oops, we should add back the old method then.
The reason is that the default startup option is RECOVER. It probably is better to change the default to FORMAT. I am fine if we revert this from branch-2 and release 2.4.0. A workaround for the bug is to specify exactly one storage dir in the conf. |
|
Filed RATIS-1694. |
|
@captainzmc , RATIS-1694 is now merged. Could you test if it could fix the problem so that we can include RATIS-1677 in 2.4.0? |
|
@szetszwo Sure, let me verify that. |
Hi @szetszwo, It appears that RATIS-1694 did not solve the CI error problem. |
|
@captainzmc , the number of failures was decreased dramatically. It is expected to have test failures. The tests depend on the auto-format behavior need to be updated. As an example, see https://github.com/captainzmc/hadoop-ozone/runs/8182884310?check_suite_focus=true#step:6:2010 . Note that this is a fix for a data loss case caused by the auto-format behavior. So I suggest to include this in 2.4.0 and then update Ozone. What do you think? |
Thanks @szetszwo for the tip, I locally change the behavior of the failed CI during init and startup RaftServer. Now this UT was successful. |
|
@captainzmc , thanks a lot for your efforts on fixing the tests! |
|
Hi @szetszwo , recently I have been checking all failed UTs and trying to fix a few. I found that almost every UT related to OM, SCM, and Datanode restart needs to fix. Init, startup, and restart of these components are all affected. Many of the default behaviors need to be changed.(Other project, such as Alluxio, may also face these issues) I wonder if we can avoid these effects by deciding in Ratis whether it need Format or RECOVER. For example, use RECOVER if the directory exists, and use format if the directory does not exist. Of course, this is just a simple idea, there must be some details to pay attention to. And what do you think here? |
@captainzmc , that is a nice idea. Unfortunately, when no directories are found, Ratis cannot decide whether a disk with an storage directory is bad or there are indeed no existing storage directories. Note that, prior this change, the RECOVER option worked exactly like that -- if a directory is found, start with it; otherwise, choose a new directory to format. Then, we discovered the problem described in HDDS-7103. When I tried to fix it, my first attempt was RATIS-1668. But then I realized that the fixed may lead to data loss described in the JIRA of this pull request. In HDFS, Namenode requires FORMAT before starting up the first time. It is the same thing. If it is too much work for the upstream applications (e.g. Ozone, Alluxio, etc), we may exclude this in 2.4.0. BTW, since the bug does not exist when there is only one directory, a workaround is NOT to configure multiple directories in |
|
Thanks @szetszwo for the explanation. Ozone will use Ratis 2.4.0 cut 1.3.0 release branch. Without sufficient testing, new problems may be introduced. I agree that exclude this commit in 2.4.0. We can support this in Ozone's Master branch so that we will have more time for adaptation and testing. |
|
Thanks @captainzmc for helping test Ratis 2.4.0. Since introduction RATIS-1677 and RATIS-1694 will indeed make Ozone, Alluxio and other components change a lot, I also agree to introduce them in the next release. Since Alluxio really wants to introduce RATIS-1695 in 2.4.0, can we start the next 2.4.0 release(2.4.0-rc2) after it's been merged? Also, do you think there is any other commit we have to introduce in 2.4.0? @szetszwo |
This reverts commit 7a4543b.
### What changes were proposed in this pull request? Bump Ratis version from 2.5.1 to 3.0.1. Address incompatible changes: - RATIS-589. Eliminate buffer copying in SegmentedRaftLogOutputStream.(apache/ratis#964) - RATIS-1677. Do not auto format RaftStorage in RECOVER.(apache/ratis#718) - RATIS-1710. Refactor metrics api and implementation to separated modules. (apache/ratis#749) ### Why are the changes needed? Bump Ratis version from 2.5.1 to 3.0.1. Ratis has released v3.0.0, v3.0.1, which release note refers to [3.0.0](https://ratis.apache.org/post/3.0.0.html), [3.0.1](https://ratis.apache.org/post/3.0.1.html). The 3.0.x version include new features like pluggable metrics and lease read, etc, some improvements and bugfixes including: - 3.0.0: Change list of ratis 3.0.0 In total, there are roughly 100 commits diffing from 2.5.1 including: - Incompatible Changes - RaftStorage Auto-Format - RATIS-1677. Do not auto format RaftStorage in RECOVER. (apache/ratis#718) - RATIS-1694. Fix the compatibility issue of RATIS-1677. (apache/ratis#731) - RATIS-1871. Auto format RaftStorage when there is only one directory configured. (apache/ratis#903) - Pluggable Ratis-Metrics (RATIS-1688) - RATIS-1689. Remove the use of the thirdparty Gauge. (apache/ratis#728) - RATIS-1692. Remove the use of the thirdparty Counter. (apache/ratis#732) - RATIS-1693. Remove the use of the thirdparty Timer. (apache/ratis#734) - RATIS-1703. Move MetricsReporting and JvmMetrics to impl. (apache/ratis#741) - RATIS-1704. Fix SuppressWarnings(“VisibilityModifier”) in RatisMetrics. (apache/ratis#742) - RATIS-1710. Refactor metrics api and implementation to separated modules. (apache/ratis#749) - RATIS-1712. Add a dropwizard 3 implementation of ratis-metrics-api. (apache/ratis#751) - RATIS-1391. Update library dropwizard.metrics version to 4.x (apache/ratis#632) - RATIS-1601. Use the shaded dropwizard metrics and remove the dependency (apache/ratis#671) - Streaming Protocol Change - RATIS-1569. Move the asyncRpcApi.sendForward(..) call to the client side. (apache/ratis#635) - New Features - Leader Lease (RATIS-1864) - RATIS-1865. Add leader lease bound ratio configuration (apache/ratis#897) - RATIS-1866. Maintain leader lease after AppendEntries (apache/ratis#898) - RATIS-1894. Implement ReadOnly based on leader lease (apache/ratis#925) - RATIS-1882. Support read-after-write consistency (apache/ratis#913) - StateMachine API - RATIS-1874. Add notifyLeaderReady function in IStateMachine (apache/ratis#906) - RATIS-1897. Make TransactionContext available in DataApi.write(..). (apache/ratis#930) - New Configuration Properties - RATIS-1862. Add the parameter whether to take Snapshot when stopping to adapt to different services (apache/ratis#896) - RATIS-1930. Add a conf for enable/disable majority-add. (apache/ratis#961) - RATIS-1918. Introduces parameters that separately control the shutdown of RaftServerProxy by JVMPauseMonitor. (apache/ratis#950) - RATIS-1636. Support re-config ratis properties (apache/ratis#800) - RATIS-1860. Add ratis-shell cmd to generate a new raft-meta.conf. (apache/ratis#901) - Improvements & Bug Fixes - Netty - RATIS-1898. Netty should use EpollEventLoopGroup by default (apache/ratis#931) - RATIS-1899. Use EpollEventLoopGroup for Netty Proxies (apache/ratis#932) - RATIS-1921. Shared worker group in WorkerGroupGetter should be closed. (apache/ratis#955) - RATIS-1923. Netty: atomic operations require side-effect-free functions. (apache/ratis#956) - RaftServer - RATIS-1924. Increase the default of raft.server.log.segment.size.max. (apache/ratis#957) - RATIS-1892. Unify the lifetime of the RaftServerProxy thread pool (apache/ratis#923) - RATIS-1889. NoSuchMethodError: RaftServerMetricsImpl.addNumPendingRequestsGauge apache/ratis#922 (apache/ratis#922) - RATIS-761. Handle writeStateMachineData failure in leader. (apache/ratis#927) - RATIS-1902. The snapshot index is set incorrectly in InstallSnapshotReplyProto. (apache/ratis#933) - RATIS-1912. Fix infinity election when perform membership change. (apache/ratis#954) - RATIS-1858. Follower keeps logging first election timeout. (apache/ratis#894) - 3.0.1:This is a bugfix release. See the [changes between 3.0.0 and 3.0.1](apache/ratis@ratis-3.0.0...ratis-3.0.1) releases. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Cluster manual test. Closes #2480 from SteNicholas/CELEBORN-1400. Authored-by: SteNicholas <programgeek@163.com> Signed-off-by: Shuang <lvshuang.xjs@alibaba-inc.com>
4 participants
See https://issues.apache.org/jira/browse/RATIS-1677