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

ES_objects plugin does not delete data from ES #2464

Closed
1 of 17 tasks
abitmore opened this issue May 18, 2021 · 4 comments · Fixed by #2570 or #2623
Closed
1 of 17 tasks

ES_objects plugin does not delete data from ES #2464

abitmore opened this issue May 18, 2021 · 4 comments · Fixed by #2570 or #2623
Assignees

Comments

@abitmore
Copy link
Member

abitmore commented May 18, 2021

Bug Description

See code:

auto obj = db.find_object(value);
auto p = static_cast<const proposal_object *>(obj);
if (p != nullptr) {
if (action == "delete")
remove_from_database(p->id, "proposal");
else
prepareTemplate<proposal_object>(*p, "proposal");
}

The code is buggy, because at first it tries to find the object from the (internal) object database via object ID, only if found, it then talks to (external) ES. But for objects which were removed from the object database, the first step will always return nullptr, as a result, the second step (remove_from_database()) is not reachable, thus data will never be removed from ES.

It could be a useful feature though. Some people may want to check removed data via ES. If it's the case, the unreachable code can be removed, or be fixed by adding an explicit "do-not-remove" option. The es-objects-keep-only-current option exists for this purpose.

Actually, by now, only proposals and limit orders are affected, because we don't delete other data. Limit orders in ES use a lot of disk space (88.9 GB at the time of writing). Proposals are 29.8 MB. I think it is better to have a dedicated option for each of them.

Update: It seems the es-objects-keep-only-current option has more use than only deleting deleted objects. Actually it tracks every change, which can be used for tracking e.g. changes on price feeds. I still think it's better to have a dedicated parameter or a set of parameters for each object type.

#808 is related.

Impacts
Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.

  • API (the application programming interface)
  • Build (the build process or something prior to compiled code)
  • CLI (the command line wallet)
  • Deployment (the deployment process after building such as Docker, Travis, etc.)
  • DEX (the Decentralized EXchange, market engine, etc.)
  • P2P (the peer-to-peer network for transaction/block propagation)
  • Performance (system or user efficiency, etc.)
  • Protocol (the blockchain logic, consensus, validation, etc.)
  • Security (the security of system or user data, etc.)
  • UX (the User Experience)
  • Other (please add below) : es_objects plugin

CORE TEAM TASK LIST

  • Evaluate / Prioritize Bug Report
  • Refine User Stories / Requirements
  • Define Test Cases
  • Design / Develop Solution
  • Perform QA/Testing
  • Update Documentation
@abitmore
Copy link
Member Author

abitmore commented Dec 28, 2021

It seems the es-objects-keep-only-current option has more use than only deleting deleted objects. Actually it tracks every change, which can be used for tracking e.g. changes on price feeds. I still think it's better to have a dedicated parameter or a set of parameters for each object type. OP updated.

@abitmore
Copy link
Member Author

Fixed by #2570.

@abitmore
Copy link
Member Author

abitmore commented Jan 6, 2022

es_objects_plugin_impl::sync_db() does not delete data from ES.

@abitmore
Copy link
Member Author

abitmore commented Aug 8, 2022

Fixed by #2570 and #2623.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment