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

HaloDBInternal delete not respecting open/close status #3

Open
sydbarrett77 opened this issue Aug 17, 2018 · 3 comments
Open

HaloDBInternal delete not respecting open/close status #3

sydbarrett77 opened this issue Aug 17, 2018 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@sydbarrett77
Copy link

Performing a delete operation on an item in a db object that is already in the closed state should throw an exception. Currently this does not happen for the first occurrence of an attempted delete, since the Tombstone file has not yet been created.

A possible solution could be to always initialize a tombstone file whenever a new db file is created.

@amannaly amannaly added the bug Something isn't working label Aug 17, 2018
@durgaswaroop
Copy link

durgaswaroop commented Aug 22, 2018

@amannaly, I would like to take this issue to fix it. How should I proceed?

One simple way is to check for dbInternal.isClosing() inside the delete() in the HaloDB class and if it is closing we can just throw the appropriate exception. Or better yet, we can set an isClosing flag in HaloDB itself and use that to figure out if the db is already closed.

The other way could be what @siddharths suggested.

Which approach would be better?

@durgaswaroop
Copy link

durgaswaroop commented Aug 22, 2018

Also, similar to this issue, I've noticed that the get operation succeeds on a closed db object for a previously unknown key but fails for a key that was present in the db.

HaloDB db = // db initialized

db.put("hello".getBytes(), "world".getBytes())

db.close()

db.get("hello".getBytes()) // ---> Throws com.oath.halodb.HaloDBException: Lookup failed.
db.get("new_key".getBytes()) // ---> Succeeds and returns 'null'

This has to be a bug as well.

@amannaly
Copy link
Collaborator

@durgaswaroop Thank you for your interest in HaloDB.

@siddharths who found this issue is already working on a fix.

We should not allow the db state to be mutated once it has been closed. This issue now happens only when a new write file or tombstone file is created, which can happen under some special cases:

  • if we open the db and close it without doing any put or delete
  • if the write file had reached the max size and will be rolled over for the next write.

I think a simple solution is to check the isClosing flag while rolling over to a new file.

I don't think we need to make any changes for the get operation as it doesn't change the db state.

If you find any other issues please feel free to create an issue here. Contributions are welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants