diff --git a/src/rgw/rgw_rest_s3vector.cc b/src/rgw/rgw_rest_s3vector.cc index a08c5231fbd..7481068a129 100644 --- a/src/rgw/rgw_rest_s3vector.cc +++ b/src/rgw/rgw_rest_s3vector.cc @@ -420,12 +420,14 @@ private: ldpp_dout(this, 1) << "ERROR: failed to load s3vector bucket " << bucket_id << ". error: " << op_ret << dendl; return; } + op_ret = rgw::s3vector::delete_vector_bucket(configuration, driver, s->user.get(), &s->bucket_tenant, this, y); + if (op_ret < 0) { + return; + } op_ret = bucket->remove(this, false, y); if (op_ret < 0) { ldpp_dout(this, 1) << "ERROR: failed to delete s3vector bucket " << bucket_id << ". error: " << op_ret << dendl; - return; } - op_ret = rgw::s3vector::delete_vector_bucket(configuration, driver, s->user.get(), &s->bucket_tenant, this, y); } }; diff --git a/src/rgw/rgw_s3vector.cc b/src/rgw/rgw_s3vector.cc index 1e149403ca1..26723219cf9 100644 --- a/src/rgw/rgw_s3vector.cc +++ b/src/rgw/rgw_s3vector.cc @@ -1294,15 +1294,26 @@ namespace rgw::s3vector { ctx->result = -EIO; return; } - // force delete TODO: fail deletion if tables exists, unless a "force" flag is used + // a vector bucket that still has indexes cannot be deleted + char** table_names; + size_t name_count; char* error_message; - if (LanceDBError err = lancedb_connection_drop_all_tables(conn, nullptr, &error_message); err != LANCEDB_SUCCESS) { - ldpp_dout(dpp, 1) << "ERROR: failed to drop content of s3vector bucket in: " << bucket_name << ". error: " << error_message << dendl; + if (const LanceDBError err = lancedb_connection_table_names(conn, &table_names, &name_count, &error_message); err != LANCEDB_SUCCESS) { + ldpp_dout(dpp, 1) << "ERROR: failed to list indexes of s3vector bucket: " << bucket_name << ". error: " << error_message << dendl; lancedb_free_string(error_message); lancedb_connection_free(conn); - ctx->result = -EIO; + ctx->result = lancedb_error_to_errno(err); return; } + if (name_count > 0) { + ldpp_dout(dpp, 1) << "ERROR: s3vector bucket: " << bucket_name << " cannot be deleted, it still has " << + name_count << " indexes" << dendl; + lancedb_free_table_names(table_names, name_count); + lancedb_connection_free(conn); + ctx->result = -ENOTEMPTY; + return; + } + lancedb_free_table_names(table_names, name_count); ldpp_dout(dpp, 20) << "INFO: deleting in-memory session (if it exists) for bucket: " << bucket_name << dendl; rgw::s3vector::notify_session_delete(dpp, bucket_name); lancedb_connection_free(conn); diff --git a/src/test/rgw/s3vectors/s3vector_test.py b/src/test/rgw/s3vectors/s3vector_test.py index e285f821acd..acfeb5cdfea 100644 --- a/src/test/rgw/s3vectors/s3vector_test.py +++ b/src/test/rgw/s3vectors/s3vector_test.py @@ -184,13 +184,29 @@ def _delete_s3_bucket_for_vector_bucket(bucket_name): log.warning("Failed to delete S3 bucket '%s': %s", bucket_name, str(err)) +def _delete_all_indexes(conn, bucket_name): + """ + Delete all indexes of a vector bucket. must be done before the vector bucket + itself is deleted, since deleting a vector bucket with indexes is not allowed + """ + result = conn.list_indexes(vectorBucketName=bucket_name) + for index in result['indexes']: + _ = conn.delete_index(vectorBucketName=bucket_name, indexName=index['indexName']) + + +def _delete_vector_bucket(conn, bucket_name): + """Delete a vector bucket together with all of its indexes""" + _delete_all_indexes(conn, bucket_name) + return conn.delete_vector_bucket(vectorBucketName=bucket_name) + + def _delete_all_vector_buckets(conn): result = conn.list_vector_buckets() assert result['ResponseMetadata']['HTTPStatusCode'] == 200 for bucket in result['vectorBuckets']: bucket_name = bucket['vectorBucketName'] try: - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) except conn.exceptions.ClientError as err: log.warning("Failed to delete vector bucket '%s': %s", bucket_name, str(err)) _delete_s3_bucket_for_vector_bucket(bucket_name) @@ -265,6 +281,36 @@ def test_delete_vector_bucket(): _delete_all_vector_buckets(conn) +@pytest.mark.vector_bucket_test +def test_delete_vector_bucket_with_indexes(): + conn = connection() + bucket_name = gen_bucket_name() + _ensure_s3_bucket_for_vector_bucket(bucket_name) + result = conn.create_vector_bucket(vectorBucketName=bucket_name) + assert result['ResponseMetadata']['HTTPStatusCode'] == 200 + index_name = 'test-index' + result = conn.create_index(vectorBucketName=bucket_name, indexName=index_name, dataType='float32', dimension=128, distanceMetric='euclidean') + assert result['ResponseMetadata']['HTTPStatusCode'] == 200 + # deleting a vector bucket which still has indexes should fail with a conflict + with pytest.raises(conn.exceptions.ClientError) as exc_info: + conn.delete_vector_bucket(vectorBucketName=bucket_name) + assert exc_info.value.response['ResponseMetadata']['HTTPStatusCode'] == 409 + assert exc_info.value.response['Error']['Code'] == 'BucketNotEmpty' + # the vector bucket and its index should still exist + result = conn.get_vector_bucket(vectorBucketName=bucket_name) + assert result['ResponseMetadata']['HTTPStatusCode'] == 200 + result = conn.get_index(vectorBucketName=bucket_name, indexName=index_name) + assert result['ResponseMetadata']['HTTPStatusCode'] == 200 + # once the index is deleted, the vector bucket could be deleted + result = conn.delete_index(vectorBucketName=bucket_name, indexName=index_name) + assert result['ResponseMetadata']['HTTPStatusCode'] == 200 + result = conn.delete_vector_bucket(vectorBucketName=bucket_name) + assert result['ResponseMetadata']['HTTPStatusCode'] == 200 + _delete_s3_bucket_for_vector_bucket(bucket_name) + # cleanup + _delete_all_vector_buckets(conn) + + @pytest.mark.vector_bucket_test def test_list_vector_buckets(): conn = connection() @@ -492,7 +538,7 @@ def test_create_index(): invalid_bucket_name = bucket_name + '-invalid' pytest.raises(conn.exceptions.ClientError, conn.create_index, vectorBucketName=invalid_bucket_name, indexName=index_name, dataType='float32', dimension=128, distanceMetric='euclidean') # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -556,7 +602,7 @@ def test_create_index_invalid_filterable_keys(): }, **common) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -585,7 +631,7 @@ def test_get_index(): invalid_bucket_name = bucket_name + '-invalid' pytest.raises(conn.exceptions.ClientError, conn.get_index, vectorBucketName=invalid_bucket_name, indexName=index_name) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -626,7 +672,7 @@ def test_non_filterable_metadata_keys(): log.info('test_non_filterable_metadata_keys: verified nonFilterableMetadataKeys round-trip') # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -733,7 +779,7 @@ def test_filterable_metadata_keys(): log.info('test_filterable_metadata_keys: verified filterable metadata round-trip') # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -842,7 +888,7 @@ def test_filterable_metadata_list_keys(): log.info('test_filterable_metadata_list_keys: verified list-type filterable metadata round-trip') # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -871,7 +917,7 @@ def test_metadata_dots_in_names_rejected(): metadataConfiguration={'nonFilterableMetadataKeys': ['user.name']}) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -896,7 +942,7 @@ def test_delete_index(): with pytest.raises(conn.exceptions.ClientError): result = conn.get_index(vectorBucketName=bucket_name, indexName=index_name) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.index_test @@ -924,7 +970,7 @@ def test_list_indexes(): invalid_bucket_name = bucket_name + '-invalid' pytest.raises(conn.exceptions.ClientError, conn.list_indexes, vectorBucketName=invalid_bucket_name) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) def assert_put_vectors_validation_error(conn, expected_paths, **kwargs): @@ -1149,7 +1195,7 @@ def test_put_vectors(): result = conn.put_vectors(vectorBucketName=bucket_name, indexName=index_name, vectors=vectors) assert result['ResponseMetadata']['HTTPStatusCode'] == 200 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) def update_vectors_thread(conn, bucket_name, index_name, dimension, allow_extra=False): @@ -1194,7 +1240,7 @@ def test_update_vectors(): t.join() # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) @pytest.mark.vector_test @@ -1218,7 +1264,7 @@ def test_update_vectors_single_index(): t.join() # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1257,7 +1303,7 @@ def test_put_vectors_dimension_mismatch(): assert result['ResponseMetadata']['HTTPStatusCode'] == 200 assert len(result.get('vectors', [])) == 0 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1286,7 +1332,7 @@ def test_get_vectors(): log.info('test_get_vectors: successfully verified %d vectors with data', len(returned_vectors)) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1315,7 +1361,7 @@ def test_get_vectors_without_data(): len(returned_vectors)) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1347,7 +1393,7 @@ def test_list_vectors(): len(all_retrieved_vectors), page_count) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1378,7 +1424,7 @@ def test_list_vectors_with_data(): len(all_retrieved_vectors)) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1409,7 +1455,7 @@ def test_list_vectors_without_data(): len(all_retrieved_vectors)) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1442,7 +1488,7 @@ def test_list_vectors_pagination(): len(all_retrieved_vectors), page_count) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1475,7 +1521,7 @@ def test_list_vectors_exact_pagination(): len(all_retrieved_vectors), page_count) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1516,7 +1562,7 @@ def test_delete_vectors(): f"remaining vector keys don't match expected. got {set(remaining_keys)}, expected {set(expected_remaining_keys)}" # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1546,7 +1592,7 @@ def test_query_vectors(): log.info(result['vectors']) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1577,7 +1623,7 @@ def test_query_vectors_with_distance(): log.info(result['vectors']) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1642,7 +1688,7 @@ def test_put_and_get_vectors_metadata(): log.info('test_put_and_get_vectors_metadata: verified metadata for %d vectors', len(all_vectors)) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1685,7 +1731,7 @@ def test_list_vectors_with_metadata(): f"vector {vector['key']} should not have 'metadata' field when returnMetadata=False" # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1731,7 +1777,7 @@ def test_query_vectors_with_metadata(): f"vector {vector['key']} should not have 'metadata' field when returnMetadata=False" # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1787,7 +1833,7 @@ def test_put_vectors_malformed_metadata(): assert len(result.get('vectors', [])) == 0 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1840,7 +1886,7 @@ def test_put_vectors_null_metadata_value(): assert len(result.get('vectors', [])) == 0 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1867,7 +1913,7 @@ def test_put_vectors_dots_in_metadata_field_names(): vectorBucketName=bucket_name, indexName=index_name, vectors=vectors) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -1957,7 +2003,7 @@ def test_put_vectors_missing_filterable_fields(): assert md['info']['year'] == 1990 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2038,7 +2084,7 @@ def test_put_vectors_invalid_filterable_types(): }]) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2171,7 +2217,7 @@ def test_put_vectors_must_exist(): assert len(result['vectors']) == 2 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2284,7 +2330,7 @@ def test_query_vectors_filter(): filter={'$or': [{'genre': 'rock'}, {'color': 'red'}]}) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2347,7 +2393,7 @@ def test_query_vectors_post_filtering(): assert query_keys({'$or': [{'genre': 'rock'}, {'color': 'blue'}]}, top_k=10, post_filtering=True) == ['v0', 'v1', 'v3'] # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2382,7 +2428,7 @@ def test_query_vectors_filter_nonfilterable(): assert exc_info.value.response['ResponseMetadata']['HTTPStatusCode'] == 400 # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2432,7 +2478,7 @@ def test_query_vectors_filter_json_metadata(): assert query_keys({'active': True}) == ['v0', 'v2'] # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2503,7 +2549,7 @@ def test_query_vectors_filter_errors(): expect_error({'color': ['red', 'blue']}) # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @pytest.mark.vector_test @@ -2565,7 +2611,7 @@ def test_query_vectors_post_filter_topk(): assert 'distance' not in v, "distance should not be in response when not requested" # cleanup - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) set_rgw_config_option('rgw_s3vector_topk_post_filter_factor', 1) @@ -2613,7 +2659,7 @@ def test_sal_error_propagation(): # cleanup - recreate backing bucket so delete_vector_bucket can clean up _ensure_s3_bucket_for_vector_bucket(bucket_name) - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @@ -2674,7 +2720,7 @@ def test_cross_owner_vector_bucket(): assert len(result['vectors']) == 2 # cleanup - _ = other.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(other, bucket_name) _delete_s3_bucket_for_vector_bucket(bucket_name) @@ -2736,7 +2782,7 @@ def test_versioned_s3_bucket(): assert len(result['vectors']) == 0 # cleanup - delete vector bucket, then purge all versions and delete markers - _ = conn.delete_vector_bucket(vectorBucketName=bucket_name) + _ = _delete_vector_bucket(conn, bucket_name) paginator = s3conn.get_paginator('list_object_versions') for page in paginator.paginate(Bucket=bucket_name): objects = []