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

feat: add majority with journaled option on write concern option #1331

Conversation

jinwoo1225
Copy link

@jinwoo1225 jinwoo1225 commented Jul 24, 2023

Summary

Add WriteConcern Option MajorityWithJournaled to explicitly sets write majority with journaled write.

Background & Motivation

To explicitly use write majority with journaled option on mongo go driver package we have to use like this.

but using two option into one struct seems awkward to me. So I propose MajorityWithJounaled function to use both option at once.

writeConcern := &writeconcern.WriteConcern{
	W:       writeconcern.Majority().W,
	Journal: writeconcern.Journaled().Journal,
}

@jinwoo1225 jinwoo1225 marked this pull request as ready for review July 24, 2023 12:40
@jinwoo1225 jinwoo1225 requested a review from a team as a code owner July 24, 2023 12:40
@jinwoo1225 jinwoo1225 requested review from matthewdale and removed request for a team July 24, 2023 12:40
@matthewdale
Copy link
Collaborator

Hey @jinwoo1225 thanks for the PR! I have a few questions:

  1. What is your use case for explicitly enabling journaling? Typically write concern majority defaults to journaling enabled unless it's disabled by writeConcernMajorityJournalDefault.
  2. How often do you declare write concerns? If it's not often, would an alternative like this work?
writeConcern := writeconcern.Majority()
j := true
writeConcern.Journal = &j

@jinwoo1225
Copy link
Author

Hello! @matthewdale Thank you for your response. and sorry for my late reply.

  1. I explicitly enabled journaling in my MongoDB configuration to ensure data durability and prevent data loss in the event of unexpected crashes or failures. While I understand that write concern majority generally defaults to journaling enabled, I wanted to explicitly specify it in my use case for added data safety and consistency.
  2. Mostly, I declare write concerns once for specific collection configurations in our application. For this purpose, I use code snippets like the following in codeblock.
writeConcern := &writeconcern.WriteConcern{
	W:       writeconcern.Majority().W,
	Journal: writeconcern.Journaled().Journal,
}

@matthewdale
Copy link
Collaborator

Hey @jinwoo1225, thanks for the additional context! Your concern about the awkward API is definitely valid. However, I don't think that adding convenience functions for more permutations of write concerns configs is a sustainable pattern. Instead, we should find a more usable way to enable journaling for any write concern and make defining struct literals for the "majority" write concern easier.

I've created GODRIVER-2953 to request an improved API for enabling journaling on write concerns and GODRIVER-2954 to request an improved way to define write concern "majority" struct literals.

Closing this PR.

@jinwoo1225
Copy link
Author

Thank you for your attention to this pull request. I'm delighted to see an enhanced version of it. 😄

@jinwoo1225 jinwoo1225 deleted the feat/add_majority_with_journaled_option_on_write_concern branch August 23, 2023 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants