aerospike / aerospike-client-java

Aerospike Java Client Library
Other
236 stars 212 forks source link

Add indexExists operation #149

Closed Aloren closed 4 years ago

Aloren commented 4 years ago

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

BrianNichols commented 4 years ago

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

BrianNichols commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.