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 indexExists operation #149

Closed

Conversation

Aloren
Copy link
Contributor

@Aloren Aloren commented Nov 26, 2019

No description provided.

@Aloren
Copy link
Contributor Author

Aloren commented Nov 26, 2019

@BrianNichols test module currently does not have any index tests. Do you mind if I add basic index create-drop-exists tests?

@BrianNichols
Copy link
Member

A basic index drop/create/drop/create test will appear in the next client release.

@BrianNichols
Copy link
Member

I'm reluctant to add a separate indexExists method because the implementation needs to be different when checking for create index versus drop index completion status.

The dropIndex task should be considered complete when the index is fully removed on all nodes. The proposed indexExists will return false (complete) when the index is marked inactive (caused by dropIndex), but has not been fully removed yet. This particular problem will be resolved by using a new sindex-exists info command that checks for full index removal in IndexTask.

The createIndex task should be considered complete when the index exists and the index stats show 100% completion on all nodes. The proposed indexExists will return true if the index exists, but may not be 100% built and thus not ready to be used. So indexExists alone is not useful for createIndex completion tests.

Also, the proposed indexExists is called on a single node. All nodes need to be checked for index completion.

@Aloren
Copy link
Contributor Author

Aloren commented Dec 3, 2019

Our use case for indexExists operation is that our services install all needed indexes on startup. By doing that automatically we do not need to have manual operations during deployment.
Currently our scenario is something like that:

if(indexExists(namespace, set, index-name)) {
  return; // index already installed
} else {
  installIndex(namespace, set, index-name, binName, type);
}

Checking for index is more lightweight operation than installing an index and then catching exception IMHO. What do you think?

@BrianNichols
Copy link
Member

installIndex is always called on a single node. That server node then sends the index creation request to all other nodes.

The sever indexExists implementation only checks it's node for the index and does not propagate the indexExists query to other nodes. Therefore, the client would really need to call indexExists on all nodes. From a client perspective, this makes indexExists(all nodes) more heavyweight than just calling installIndex(single node) and handling the exception with the INDEX_ALREADY_EXISTS result code.

@kptfh
Copy link

kptfh commented Dec 6, 2019

Usually all communication related to infrastructure (like index create/exist) is negligible compared to huge numbers of CRUDs. So I think we may pay less attention to performance aspects of such operations.

@Aloren
Copy link
Contributor Author

Aloren commented Dec 6, 2019

I suppose it is better to go with createIndex operation and handle already-exists error, as it is more light weight both for the Aerospike server and more simple in implementation from client perspective. Thank you @BrianNichols for the explanation.

@Aloren Aloren closed this Dec 6, 2019
@Aloren Aloren deleted the feature/index-exists-operation branch December 6, 2019 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants