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 online monitor bug for only md stored #596

Merged
merged 4 commits into from Dec 9, 2021

Conversation

JoranAngevaare
Copy link
Member

What is the problem / what does the code in this PR do
Sometimes, we get errors that data cannot be loaded from the MongoFrontend when only the metadata is written. This PR adds a test that was failing (before making the changes in the MongoFrontend).

Can you briefly describe how it works?
Check that a chunk of data is written rather than only the metadata document

Can you give a minimal working example (or illustrate with a figure)?
Please see the tests

Comment on lines -184 to +194
query = backend_key_to_query(str(key))
if self.db[self.col_name].count_documents(query):
self.log.debug(f"{key} is in cache.")
# Should have at least one non-metadata chunk, otherwise there
# is no data to load
query = {**backend_key_to_query(str(key)),
'provides_meta': False
}
if self.collection.count_documents(query):
self.log.debug(f"{key} is in database.")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the fix

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternately the same query would work but we check that count_documents() returns at least 2

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, this is true and actually what I did first (when I was trying to isolate the issue). Due to another bug (in my old version of the test) it did not seem to work. I could now change now, but I guess there is not much of a difference now.

Comment on lines +209 to +217
# Now check that both frontends understand there is no data (even when
# allow_incomplete is set)
stored_in_st = self.is_stored_in_mongo
stored_in_incomplete_st = self.stored_in_context(st_incomplete_allowed)
if stored_in_st or stored_in_incomplete_st:
raise RuntimeError(f'Only metadata written and should NOT stored in st '
f'({stored_in_st}) and st_incomplete ('
f'{stored_in_incomplete_st}) '
f'\n{self.chunk_summary}')
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the check that failed before and is now fixed

@@ -160,7 +160,7 @@ def _clean_first_key_from_registry(self):
class MongoFrontend(StorageFrontend):
"""MongoDB storage frontend"""

def __init__(self, uri, database, col_name=None, *args, **kwargs):
def __init__(self, uri, database, col_name, *args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

No type hints?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, let me add some

Comment on lines -184 to +194
query = backend_key_to_query(str(key))
if self.db[self.col_name].count_documents(query):
self.log.debug(f"{key} is in cache.")
# Should have at least one non-metadata chunk, otherwise there
# is no data to load
query = {**backend_key_to_query(str(key)),
'provides_meta': False
}
if self.collection.count_documents(query):
self.log.debug(f"{key} is in database.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Alternately the same query would work but we check that count_documents() returns at least 2

Copy link
Contributor

@jmosbacher jmosbacher left a comment

Choose a reason for hiding this comment

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

Looks good.

strax/storage/mongo.py Show resolved Hide resolved
strax/storage/mongo.py Show resolved Hide resolved
@JoranAngevaare JoranAngevaare added the Testing Works on testing code label Nov 29, 2021
@WenzDaniel
Copy link
Collaborator

Since approved I merge to get ready for release.

@WenzDaniel WenzDaniel merged commit 60c3a07 into master Dec 9, 2021
@WenzDaniel WenzDaniel deleted the not_stored_when_only_md branch December 9, 2021 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Testing Works on testing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants