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.

Sign up for GitHub

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

Jump to bottom

[ALLUXIO-3392] embedded journal #8219

Merged
merged 8 commits into from
Dec 28, 2018
Merged

[ALLUXIO-3392] embedded journal #8219

merged 8 commits into from
Dec 28, 2018

Conversation

LuQQiu
Copy link
Contributor

@LuQQiu LuQQiu commented Dec 27, 2018

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1298/
Test FAILed.

aaudiber
aaudiber suggested changes Dec 27, 2018
View reviewed changes
core/common/src/main/java/alluxio/master/MasterInquireClient.java Outdated
@@ -87,6 +87,9 @@ public static MasterInquireClient create(AlluxioConfiguration conf) {
return ZkMasterInquireClient.getClient(conf.get(PropertyKey.ZOOKEEPER_ADDRESS),
conf.get(PropertyKey.ZOOKEEPER_ELECTION_PATH),
conf.get(PropertyKey.ZOOKEEPER_LEADER_PATH));
} else if (alluxio.util.ConfigurationUtils.getMasterRpcAddresses(conf).size() > 1) {
return new PollingMasterInquireClient(
alluxio.util.ConfigurationUtils.getMasterRpcAddresses(conf));
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use just ConfigurationUtils instead of the fully-qualified path alluxio.util.ConfigurationUtils. Same for the rest of the PR - unless there is a conflict with another class in the file, we don't use fully-qualified paths

core/protobuf/src/proto/journal/journal.proto Outdated
@@ -35,10 +36,14 @@ message JournalEntry {
optional DeleteLineageEntry delete_lineage = 7;
optional DeleteMountPointEntry delete_mount_point = 8;
optional DeleteStoreEntry delete_store = 25;
optional FinishJobEntry finish_job = 39;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

core/protobuf/src/proto/journal/journal.proto Outdated
@@ -53,6 +58,7 @@ message JournalEntry {
optional RenameStoreEntry rename_store = 28;
optional SetAclEntry set_acl = 31;
optional SetAttributeEntry set_attribute = 27;
optional StartJobEntry start_job = 41;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

core/server/common/src/main/java/alluxio/master/journal/raft/AnchoredFixedRateRunnable.java Outdated
* will run at 4am, noon, and 8pm every day. The anchor time avoids issues with clock drift and
* rounding errors.
*/
public class AnchoredFixedRateRunnable implements Runnable {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this file used anywhere?

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1311/
Test FAILed.

@LuQQiu
Copy link
Contributor Author

LuQQiu commented Dec 27, 2018

jenkins, test this please

@Alluxio Alluxio deleted a comment from AmplabJenkins Dec 27, 2018
@Alluxio Alluxio deleted a comment from AmplabJenkins Dec 27, 2018
@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1313/
Test FAILed.

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1322/
Test FAILed.

@AmplabJenkins
Copy link

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1323/
Test FAILed.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1325/
Test PASSed.

witgo
witgo reviewed Dec 28, 2018
View reviewed changes
pom.xml
@@ -507,6 +512,22 @@
<artifactId>httpcore</artifactId>
<version>4.4.6</version>
</dependency>
<dependency>
<groupId>io.atomix.copycat.alluxio</groupId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use the latest version of atomix ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree that we should upgrade to the latest version at some point, but the Atomix 3 API is very different from the Copycat API, so it will take some serious effort. I think it's worth merging with the Copycat API for now, then upgrading later. Copycat was stable for a long time before the Atomix refactor.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1332/
Test PASSed.

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Alluxio-Pull-Request-Builder/1338/
Test PASSed.

aaudiber
aaudiber approved these changes Dec 28, 2018
View reviewed changes
Copy link
Contributor

@aaudiber aaudiber left a comment

Choose a reason for hiding this comment

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

LGTM

@LuQQiu LuQQiu merged commit 6eaf937 into Alluxio:master Dec 28, 2018
@LuQQiu LuQQiu mentioned this pull request Dec 28, 2018
Merged
@kaijchen kaijchen mentioned this pull request Mar 1, 2023
Merged
alluxio-bot pushed a commit that referenced this pull request Mar 2, 2023
@kaijchen
[DOCFIX] Correct docs about Ratis
49a0a55 
### What changes are proposed in this pull request?

Correct docs about Ratis.

### Why are the changes needed?

Embedded journal is introduced in #8219,
then Copycat is replaced by Ratis in #12181.
Some docs are not updated.

### Does this PR introduce any user facing changes?

No.

pr-link: #16985
change-id: cid-592a5c06991c5b067690031ef7fffed9d2e6fac6
YangchenYe323 pushed a commit to YangchenYe323/alluxio that referenced this pull request Apr 16, 2023
@kaijchen @YangchenYe323
[DOCFIX] Correct docs about Ratis
039c3d9 
### What changes are proposed in this pull request?

Correct docs about Ratis.

### Why are the changes needed?

Embedded journal is introduced in Alluxio#8219,
then Copycat is replaced by Ratis in Alluxio#12181.
Some docs are not updated.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16985
change-id: cid-592a5c06991c5b067690031ef7fffed9d2e6fac6
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 17, 2023
@kaijchen @jiacheliu3
[DOCFIX] Correct docs about Ratis
6f06dfe 
### What changes are proposed in this pull request?

Correct docs about Ratis.

### Why are the changes needed?

Embedded journal is introduced in Alluxio#8219,
then Copycat is replaced by Ratis in Alluxio#12181.
Some docs are not updated.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16985
change-id: cid-592a5c06991c5b067690031ef7fffed9d2e6fac6
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 17, 2023
@kaijchen @jiacheliu3
[DOCFIX] Correct docs about Ratis
75179ce 
### What changes are proposed in this pull request?

Correct docs about Ratis.

### Why are the changes needed?

Embedded journal is introduced in Alluxio#8219,
then Copycat is replaced by Ratis in Alluxio#12181.
Some docs are not updated.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16985
change-id: cid-592a5c06991c5b067690031ef7fffed9d2e6fac6
jiacheliu3 pushed a commit to jiacheliu3/alluxio that referenced this pull request May 17, 2023
@kaijchen @jiacheliu3
[DOCFIX] Correct docs about Ratis
3000653 
### What changes are proposed in this pull request?

Correct docs about Ratis.

### Why are the changes needed?

Embedded journal is introduced in Alluxio#8219,
then Copycat is replaced by Ratis in Alluxio#12181.
Some docs are not updated.

### Does this PR introduce any user facing changes?

No.

pr-link: Alluxio#16985
change-id: cid-592a5c06991c5b067690031ef7fffed9d2e6fac6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@witgo witgo witgo left review comments

@aaudiber aaudiber aaudiber approved these changes

Assignees

@aaudiber aaudiber

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@LuQQiu @AmplabJenkins @witgo @aaudiber