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

Recreate logger on new term #270

Merged
merged 1 commit into from
Mar 6, 2023
Merged

Recreate logger on new term #270

merged 1 commit into from
Mar 6, 2023

Conversation

nahguam
Copy link
Contributor

@nahguam nahguam commented Mar 6, 2023

In zerolog, log keys are additive and it does not deduplicate fields in json output

In zerolog, log keys are additive and it does not deduplicate fields in json output
@andrasbeni
Copy link
Contributor

I'm surprised this does not get caught by the race condition detection of go test since we do not guard fc.log in any way. But I think it is OK if the go memory model states that assignments are atomic. Do you know if this is the case?

@nahguam
Copy link
Contributor Author

nahguam commented Mar 6, 2023

I'm surprised this does not get caught by the race condition detection of go test since we do not guard fc.log in any way. But I think it is OK if the go memory model states that assignments are atomic. Do you know if this is the case?

race only detects at runtime, so not really surprising. I've added a lock

@nahguam
Copy link
Contributor Author

nahguam commented Mar 6, 2023

I'm surprised this does not get caught by the race condition detection of go test since we do not guard fc.log in any way. But I think it is OK if the go memory model states that assignments are atomic. Do you know if this is the case?

race only detects at runtime, so not really surprising. I've added a lock

Actually, the logger is only set on construction and in a method that is already utilizing a lock, so I've reverted my last

@andrasbeni
Copy link
Contributor

Yes, construction is covered with locks, but not all reads of leaderController.log are. E.g.,func (lc *leaderController) read(... uses log without a lock. But in java it would still be fine since pointer assignments are atomic. I just don't know if they are atomic in golang.

@nahguam nahguam merged commit b9b5f80 into streamnative:main Mar 6, 2023
@nahguam nahguam deleted the dedupe-log-keys branch March 6, 2023 15:42
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.

2 participants