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

Use FileSystem sync for AOF #91

Merged
merged 1 commit into from
Dec 23, 2021
Merged

Use FileSystem sync for AOF #91

merged 1 commit into from
Dec 23, 2021

Conversation

the123saurav
Copy link
Contributor

No description provided.

@the123saurav the123saurav marked this pull request as draft December 22, 2021 13:31
@the123saurav the123saurav marked this pull request as ready for review December 22, 2021 14:57
@tuhuynh27 tuhuynh27 added the enhancement New feature or request label Dec 23, 2021
@tuhuynh27 tuhuynh27 added this to the First Release milestone Dec 23, 2021
tuhuynh27
tuhuynh27 reviewed Dec 23, 2021
View reviewed changes
core/src/main/java/dev/keva/core/aof/AOFContainer.java

@Autowired
private KevaConfig kevaConfig;

public void init() {
alwaysFlush = kevaConfig.getAofInterval() == 0;
Copy link
Member

Choose a reason for hiding this comment

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

@the123saurav Can you replace all the kevaConfig.getAofInterval() != 0 to alwaysFlush

Copy link
Member

Choose a reason for hiding this comment

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

Line 49:

if (kevaConfig.getAofInterval() != 0) {
            ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

tuhuynh27
tuhuynh27 approved these changes Dec 23, 2021
View reviewed changes
Copy link
Member

@tuhuynh27 tuhuynh27 left a comment

Choose a reason for hiding this comment

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

So far LGTM

@the123saurav
Copy link
Contributor Author

@tuhuynh27
Should i wait for more approvals?

@tuhuynh27 tuhuynh27 merged commit 9a80c8d into develop Dec 23, 2021
@tuhuynh27 tuhuynh27 deleted the aof-perf branch December 23, 2021 11:23
@tuhuynh27 tuhuynh27 mentioned this pull request Dec 23, 2021
Merged
tuhuynh27 pushed a commit that referenced this pull request Dec 23, 2021
@the123saurav @tuhuynh27
uses FileSystem sync for AOF (#91)
f8425d0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@axblueblader axblueblader Awaiting requested review from axblueblader

@haphananhtuan haphananhtuan Awaiting requested review from haphananhtuan

@yampml yampml Awaiting requested review from yampml

@tuhuynh27 tuhuynh27 Awaiting requested review from tuhuynh27

Assignees

@the123saurav the123saurav

Labels
enhancement New feature or request
Projects
None yet
Milestone
First Release
Development

Successfully merging this pull request may close these issues.
2 participants
@the123saurav @tuhuynh27