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

Add IndexManager #2879

Merged
merged 11 commits into from
Nov 27, 2018
Merged

Add IndexManager #2879

merged 11 commits into from
Nov 27, 2018

Conversation

cdonati
Copy link
Member

@cdonati cdonati commented Sep 27, 2018

The IndexManager maintains a cache of datastore index definitions that's kept up to date by ZooKeeper watches.

It also keeps track of composite index status and backfills indexes when needed.

This also introduces a slight delay for newly defined indexes while they are backfilled.

Resolves #2874
Resolves #2878

Future work in this area includes

  • Modifying clients (mostly the API servers) to not include index definitions for requests
  • Re-adding the ability to perform datastore mutations from the dashboard

This makes them accessible to both appscale-admin and
appscale-datastore.
This allows each datastore server to assume a management role
when needed without relying on a separate thread.
The IndexManager maintains a cache of datastore index definitions
that's kept up to date by ZooKeeper watches.

It also keeps track of composite index status and backfills
indexes when needed.
@sjones4
Copy link
Contributor

sjones4 commented Oct 19, 2018

I think the life cycle of the index metadata no longer matches that of the index data. If you undeploy and then deploy an application the index metadata will be recreated and the index data will then be duplicated.

@cdonati
Copy link
Member Author

cdonati commented Oct 19, 2018

@sjones4 That's a good point. Given that we don't delete a project's datastore data on undeploy, it seems that an undeploy on AppScale is more similar to disabling the project than deleting it.

Do you think it would be sufficient if we just left the metadata intact on an undeploy? We could remove the services node instead of the entire project tree. An alternative would be do implement a servingStatus field on the project node.

We may also want to rename the undeploy operation to make it clearer what it does. The easiest change in the short term would probably be allowing deletions only on the service level.

@sjones4
Copy link
Contributor

sjones4 commented Oct 19, 2018

@cdonati leaving the project level metadata intact on undeploy sounds fine to me. The metadata for services would then reflect what was currently deployed.

existing_indexes = [DatastoreIndex.from_dict(project_id, index)
for index in json.loads(existing_indexes)]
except (TypeError, ValueError):
existing_indexes = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this part of a normal workflow? It's not clear why listed errors should be ignored silently. Should they?

This makes it clearer that a non-existent indexes node is not an
exceptional circumstance.
After ensure_path, the node data is an empty string.
@sjones4 sjones4 merged commit 894304b into AppScale:master Nov 27, 2018
@cdonati cdonati deleted the index-manager branch November 27, 2018 22:48
@sjones4 sjones4 added this to the 3.7.0 milestone Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants