Netflix / hollow

Hollow is a java library and toolset for disseminating in-memory datasets from a single producer to many consumers for high performance read-only access.
Apache License 2.0
1.21k stars 217 forks source link

IncrementalProducer doesn't delete orphans with new Types in deltas [bug/question?] #162

Open rpalcolea opened 6 years ago

rpalcolea commented 6 years ago

HollowIncrementalProducer doesn't cleanup orphan objects if the type was not present in the first snapshot (introduced a new Type in a delta).

turns out HollowReadStateEngine.typeStates doesn't add a type once you write a delta, only when snapshot.

here is wehre type states are added

https://github.com/Netflix/hollow/blob/d4b574e5f4e941b9fd8faf9a2c52125a05f417f6/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowReadStateEngine.java#L96

and looks like

https://github.com/Netflix/hollow/blob/12f94a903a765a8a63de8046d2c846398e11d236/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowBlobReader.java#L196

So any time the incremental producer tries to delete orphan objects for types that are added in a delta, using:

https://github.com/Netflix/hollow/blob/79e6eb2f2c621ab5cccda2669f81f36de93bb594/hollow/src/main/java/com/netflix/hollow/api/producer/HollowIncrementalCyclePopulator.java#L50

to retrieve the type and determine which ordinals to remove returns a null type.

I was thinking on adding stateEngine.addTypeState(typeState) in https://github.com/Netflix/hollow/blob/12f94a903a765a8a63de8046d2c846398e11d236/hollow/src/main/java/com/netflix/hollow/core/read/engine/HollowBlobReader.java#L208 but found out that multiple instances of HollowReadStateEngine are created and HollowIncrementalCyclePopulator it's using one that doesn't have the new types.

Any suggestion for fixing this bug? I think I'm a little lost on this portion of HollowReadStateEngine. While an initial snapshot should contain most of the possible hollow types, there could be a scenario where a new type is added later in a delta chain.

rpalcolea commented 6 years ago

Opened a PR (https://github.com/Netflix/hollow/pull/160/files) to test that the orphan delete works when:

  1. an object of Hollow Type is added in the first snapshot
  2. an object of a Hollow Type is added in a delta.

The issue described above still persists (only if you don't initialize the model)

rpalcolea commented 6 years ago

Turns out that it only fails if you don't initialize your model. initializing like this -> https://github.com/Netflix/hollow/pull/160/files#diff-d3eea729923edc176ac3071f71627fe2R373

Allows to add objects of TypeC in a delta and remove orphans.

At this point, I wonder if this is a bug or just something desired for cases where you don't initialize your model, which probably you should every time?