Skip to content

Conversation

Eshanatnight
Copy link
Contributor

Fixes #615.

Description

Return the proper Error Message if the Data Store is in an invalid state.

Invalid States

  • .parseable.json does not exists but directories with .stream.json exist
  • .parseable.json does not exists but directories with .stream.json do not exist
  • .parseable.json does not exists and directories without .stream.json exists

Added a validation step to check if the Storage Location is in a valid state.

  • Add new trait in ObjectStorage: async fn list_dirs_in_storage(&self) -> Result<Vec<String>, ObjectStorageError>
  • Impl list_dirs_in_storage for localfs and s3
  • Add a validation method for checking if the Data Store is in a valid state.

This PR has:

  • [x ] been tested to ensure log ingestion and log query works.
  • [ x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.

Ok(())
} else if has_parseable_json && !has_streams {
Err(ObjectStorageError::Custom(
"Parseable config found, but the Storage contains some stale data.".to_owned(),
Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable Jan 17, 2024

Choose a reason for hiding this comment

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

please correct the error to -
Could not start the server because storage contains stale data from previous deployment, please choose an empty storage and restart the server. Join us on Parseable Slack to report this incident : https://launchpass.com/parseable

Also, Parseable.json file gets created in storage, staging directory gets created, parseable.json gets created in staging -- none of these should happen in case of error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This error will be triggered if someone added some data to the storage where parseable stores data.
This case is the least likeliest to happen.

))
} else if !has_parseable_json && !has_streams && !has_dirs {
Err(ObjectStorageError::Custom(
"Storage contains some stale data, Please provide an Empty Storage".to_owned(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Please provide the scenario when can this happen -!has_parseable_json && !has_streams && !has_dirs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The variable has_dirs does not properly convey the intention.

In this case .parseable.json, folders with .stream.json do not exists, but there is some data present in the storage.
This will happen if parseable is given a storage that is not empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am updating the logic of the if condition, so the intention is clearer.

))
} else {
Err(ObjectStorageError::Custom(
"Parseable config is missing, but streams are present in Storage.\nJoin us on Parseable Slack"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please provide the scenario when can this happen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is again a highly unlikely scenario to happen.
This error will be triggered if .parseable.json was deleted from the storage manually.

))
} else if !has_parseable_json && !has_streams && has_dirs {
Err(ObjectStorageError::Custom(
"Could not start the server because storage contains some stale data, please provide an empty storage and restart the server.\nJoin us on Parseable Slack to report this incident : launchpass.com/parseable".to_owned(),
Copy link
Contributor

Choose a reason for hiding this comment

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

@Eshanatnight please update to below error

"Could not start the server because storage indicates stale data not related to parseable, please choose an empty storage and restart the server.\nJoin us on Parseable Slack to report this incident : launchpass.com/parseable"

))
} else {
Err(ObjectStorageError::Custom(
"Could not start the server because storage contains stale data from previous deployment.\nJoin us on Parseable Slack to report this incident : launchpass.com/parseable"
Copy link
Contributor

Choose a reason for hiding this comment

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

@Eshanatnight please update to below error

"Could not start the server because storage indicates stale data from previous deployment, please choose an empty storage and restart the server.\nJoin us on Parseable Slack to report this incident : launchpass.com/parseable"

Ok(())
} else if has_parseable_json && !has_streams {
Err(ObjectStorageError::Custom(
"Could not start the server because storage contains stale data from previous deployment, please choose an empty storage and restart the server.\nJoin us on Parseable Slack to report this incident : launchpass.com/parseable"
Copy link
Contributor

Choose a reason for hiding this comment

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

@Eshanatnight please update to below error message

"Could not start the server because storage indicates stale data from previous deployment, please choose an empty storage and restart the server.\nJoin us on Parseable Slack to report this incident : launchpass.com/parseable"

Copy link
Contributor

@nikhilsinhaparseable nikhilsinhaparseable left a comment

Choose a reason for hiding this comment

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

@Eshanatnight please change the error messages as per the comments, rest all look good. I have tested the scenarios and all scenarios work as expected.
@nitisht once Eshan makes the error messages consistent, we are good to merge this PR as well.

@nitisht
Copy link
Member

nitisht commented Jan 22, 2024

Merging now, I'll fix the error message in next PR.

@nitisht nitisht merged commit 41edfe0 into parseablehq:main Jan 22, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
@Eshanatnight Eshanatnight deleted the dev branch February 25, 2024 16:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error message: log proper error when S3 bucket has pre-existing data
3 participants