codenotary / immudb

immudb - immutable database based on zero trust, SQL/Key-Value/Document model, tamperproof, data change history
https://immudb.io
Other
8.52k stars 337 forks source link

GetAll is not failing under keynotfound #1350

Open emkaminsk opened 1 year ago

emkaminsk commented 1 year ago

GetAll may return a sublist of the requested entries in case keys are not found

The sublist may be returned but the error of keynotfound may be returned anyways

Existent applications may expect all requested keys were found if no error is raised. While I think it's the same behaviour from the "older times", it may not be the expected one

jeroiraz commented 1 year ago

It's important to fix and test TrustCenter and CAS without waiting for the next release

emkaminsk commented 1 year ago

@qrdl @ldej Please take a look

byo commented 1 year ago

@qrdl @ldej Please take a look

We need to check all calls of GetAll() on TrustCenter (and old CNC) if it has any expectation from immudb about keys which are missing in the database. Currently we don't fail but return smaller subset of key-value-metadata pairs but we would like to throw an error in such case.

qrdl commented 1 year ago

vcn handles the situation when TC returns less assets than was requested, the missing ones are considered unknown. vcn doesn't communicate with immudb directly, but I guess TC just passes it to vcn

ldej commented 1 year ago

GetAll is used in the following places in codenotary-cloud:

  1. Data service uses it in VCNGetArtifacts which is used by vcn
  2. When querying a ledger, postgres is used as search cache. The hashes found in that query are requested from immudb with GetAll

That's it.

byo commented 1 year ago

vcn handles the situation when TC returns less assets than was requested, the missing ones are considered unknown. vcn doesn't communicate with immudb directly, but I guess TC just passes it to vcn

OK, this means that the change we want to add to immudb will not be compatible with current TC code.