aerospike / aerospike-client-java

Aerospike Java Client Library
Other
236 stars 212 forks source link

Executing query when set is not present #122

Closed Aloren closed 2 years ago

Aloren commented 5 years ago

When we execute query with a set that is not present in Aerospike server we get the following warning on server:

Nov 22 2018 10:49:53 GMT: WARNING (scan): (scan.c:273) scan msg from ip:port has unrecognized set our-set-name

We use the following code:

import com.aerospike.helper.query.KeyRecordIterator;
import com.aerospike.helper.query.QueryEngine;

    protected <T> Stream<T> findAllUsingQuery(Class<T> type, Filter filter, Qualifier... qualifiers) {
        return findAllRecordsUsingQuery(type, filter, qualifiers)
                .map(keyRecord -> mapToEntity(keyRecord.key, type, keyRecord.record));
    }

    private <T> Stream<KeyRecord> findAllRecordsUsingQuery(Class<T> type, Filter filter, Qualifier... qualifiers) {
        String setName = getSetName(type);

        KeyRecordIterator recIterator = this.queryEngine.select(
                this.namespace, setName, filter, qualifiers);

        return StreamUtils.createStreamFromIterator(recIterator)
                .onClose(() -> {
                    try {
                        recIterator.close();
                    } catch (Exception e) {
                        log.error("Caught exception while closing query", e);
                    }
                });
    }

On client we receive empty result as expected. Do we need to check for set existence before executing query or we need to file a ticket for server team to decrease log level?

gooding470 commented 5 years ago

Hi Nastya,

I will remove the warning in our next server release. I can convert it to an info or detail level log.

Strangely enough, the warning was added years ago on the insistence of a customer who had tried a scan using the incorrect set name. They saw no records returned, and it took a while for them to figure out their query had a typo in the set name. When they discovered it, they insisted we should have a warning to indicate that the set didn't exist.

It's hard to please everyone... and I don't know if this customer cares about this any more. If it's just the warning level that bothers you, we could maybe compromise and use info level (which shows up in the log by default). If you don't want it in the log at all, I'll just make it detail.

Andy

gooding470 commented 5 years ago

Hi again, Nastya,

We noticed that in this case, our scan (which is what we call a query with no 'where' clause) generates a warning and returns result code 2 ("not found"), whereas a query (with a 'where' clause) that goes through a secondary index generates an 'info' level log and returns result code 0 (OK).

This is inconsistent behavior by our server, so we're looking at also changing the result code. This is a more substantive change, and so likely won't make it into our very next hotfix release.

We'd appreciate your feedback on whether the result code in this case is important to you, or if it was only the warning that was a concern.

Andy

Aloren commented 5 years ago

Hello Andy, Thanks for the detailed answer. I can not insist on modifying the sources of Aerospike server :) But I think that returning empty result for the non-existent set -- is an issue that led to adding warning on server. I would rather return specific error code (something like SET_NOT_FOUND) so that the end user knows that input for query is not valid.

gooding470 commented 5 years ago

Ok, thanks Nastya. To be clear, you prefer a result code like "not found", but you don't want the warning. That makes sense. We'll go ahead then, and reduce the warning to a 'detail' (which doesn't show in the log by default) but we'll return a "not found" result. We'll do this for both 'scans' and 'queries', in a near future release, perhaps 4.5. Not sure yet whether we'll use the generic "not found" (2), or add a new error code for "set not found".

Aloren commented 5 years ago

Ok, good. I suppose we also need to update query-engine library to support new error codes.

gooding470 commented 5 years ago

Hi again, Nastya.

It turns out that although our server already return an error code 2 (not found) for a scan on a set that doesn't exist, the client converts this error code to 0 (ok) when it returns the result to the application. Therefore this particular server error doesn't get through to the application, and you said you would prefer if it did. We therefore need to discuss at Aerospike whether we should change the client so it does not convert the result code. (I have no problem with that in principle, but we need to investigate how tests and other customers might be impacted.)

Meanwhile we can go ahead and demote the server warning, but until we change the client your application would still get an "ok" on scanning a non-existent set. (You could of course tweak your own client for now, if it's important enough.)