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

Fix visibility in buffered translog #5609

Closed
wants to merge 1 commit into from

Conversation

@kimchy
Copy link
Member

commented Mar 30, 2014

  • fix visiblity of last written position in translog
  • while there, make sure to properly propagate the exception from sync()
- fix visiblity of last written position in translog
- while there, make sure to properly propagate the exception from sync()
raf.channel().force(false);
} catch (Exception e) {
// ignore
flushBuffer();

This comment has been minimized.

Copy link
@s1monw

s1monw Mar 31, 2014

Contributor

can we assert in flushBuffer that we hold the rwl#writeLock?

This comment has been minimized.

Copy link
@kimchy

kimchy Mar 31, 2014

Author Member

sure, will require casting to ReentrantReadWriteLock.WriteLock, but thats fine, we create that lock anyhow

}
} finally {
raf.decreaseRefCount(delete);

This comment has been minimized.

Copy link
@s1monw

s1monw Mar 31, 2014

Contributor

we decrease the ref count multiple times if we call close more than once - I think we must protect from this though

This comment has been minimized.

Copy link
@kimchy

kimchy Mar 31, 2014

Author Member

sure, can add protection. Its protected on the higher level, but makes sense to protect here as well

@@ -146,40 +146,33 @@ public boolean syncNeeded() {
}

@Override
public void sync() {
public void sync() throws IOException {
if (lastPosition == lastSyncPosition) {

This comment has been minimized.

Copy link
@s1monw

s1monw Mar 31, 2014

Contributor

can you just call syncNeeded() that way we don't have it in two places?

This comment has been minimized.

Copy link
@kimchy

kimchy Mar 31, 2014

Author Member

yes we can!

This comment has been minimized.

Copy link
@synhershko

synhershko Mar 31, 2014

Contributor

yes oui כן!

(sorry, couldn't resist...)

@kimchy

This comment has been minimized.

Copy link
Member Author

commented Mar 31, 2014

pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.