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.

Sign up for GitHub

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

Jump to bottom

compact_incomplete should only write a symbol list key if the symbol is new #1324

Closed
alexowens90 opened this issue Feb 13, 2024 · 1 comment · Fixed by #1798
Closed

compact_incomplete should only write a symbol list key if the symbol is new #1324

alexowens90 opened this issue Feb 13, 2024 · 1 comment · Fixed by #1798
Assignees
@IvoDD
Labels
enhancement New feature or request

Comments

@alexowens90
Copy link
Collaborator

alexowens90 commented Feb 13, 2024 •
edited
Loading

Currently, compact_incomplete always writes a symbol list key. This should only be written if finalize_staged_data/compact_incomplete specify that it was a parallel write, if it was a parallel append, but with an (as yet unimplemented) upsert flag set to True, or if it is being called by a tick collector and the symbol is new.
@alexowens90 alexowens90 added the bug Something isn't working label Feb 13, 2024
@alexowens90 alexowens90 added enhancement New feature or request and removed bug Something isn't working labels Jul 18, 2024
@IvoDD
Copy link
Collaborator

IvoDD commented Aug 28, 2024

Also it looks like finalize_staged_data doesn't update the symbol list at all which leads to the following bug:

    lib.write("sym", df, staged=True)
    lib.sort_and_finalize_staged_data("sym", mode=StagedDataFinalizeMethod.APPEND)
    assert lib_tool.count_keys(ArcticKeyType.SYMBOL_LIST) == 0
    assert lib.list_symbols() == ["sym"]
    assert lib_tool.count_keys(ArcticKeyType.SYMBOL_LIST) == 1

    lib.write("sym_2", df, staged=True)
    lib.sort_and_finalize_staged_data("sym_2", mode=StagedDataFinalizeMethod.APPEND)
    assert lib.list_symbols() == ["sym"]

After the first symbol list compaction we no longer look at the version keys and we fail to list sym_2 as a symbol the second time.

We should fix this alongside the compact_incomplete issue.

IvoDD added a commit that referenced this issue Aug 28, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
728623a 
- Introduces a new `update_symbol_list_on_append_` in
  `CompactIncompleteOptions`. It specifies when to update the symbol
list cache when compacting append keys.
- Introduces the `upsert_symbol_list` flag on `compact_incomplete` and
  `finalize_staged_data`. It specifies whether to add to the symbol list
when appending.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
@IvoDD IvoDD mentioned this issue Aug 28, 2024
Merged
5 tasks
IvoDD added a commit that referenced this issue Aug 28, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
96bde79 
- Introduces a new `update_symbol_list_on_append_` in
  `CompactIncompleteOptions`. It specifies when to update the symbol
list cache when compacting append keys.
- Introduces the `upsert_symbol_list` flag on `compact_incomplete` and
  `finalize_staged_data`. It specifies whether to add to the symbol list
when appending.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
IvoDD added a commit that referenced this issue Aug 28, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
5263f73 
- Introduces a new `update_symbol_list_on_append_` in
  `CompactIncompleteOptions`. It specifies when to update the symbol
list cache when compacting append keys.
- Introduces the `upsert_symbol_list` flag on `compact_incomplete` and
  `finalize_staged_data`. It specifies whether to add to the symbol list
when appending.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
IvoDD added a commit that referenced this issue Aug 28, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
521abee 
- Introduces a new `update_symbol_list_on_append_` in
  `CompactIncompleteOptions`. It specifies when to update the symbol
list cache when compacting append keys.
- Introduces the `upsert_symbol_list` flag on `compact_incomplete` and
  `finalize_staged_data`. It specifies whether to add to the symbol list
when appending.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
IvoDD added a commit that referenced this issue Sep 4, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
ca475c5 
- Only adds a symbol list entry on append if there is no previous
  version.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
IvoDD added a commit that referenced this issue Sep 5, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
e9a7aca 
- Only adds a symbol list entry on append if there is no previous
  version.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
IvoDD added a commit that referenced this issue Sep 5, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
c6a0567 
- Only adds a symbol list entry on append if there is no previous
  version.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
vasil-pashov pushed a commit that referenced this issue Sep 30, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
1dd6793 
- Only adds a symbol list entry on append if there is no previous
  version.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
vasil-pashov pushed a commit that referenced this issue Sep 30, 2024
@IvoDD
Fix symbol list bug on compaction (#1324)
c430234 
- Only adds a symbol list entry on append if there is no previous
  version.
- Fixes `finalize_staged_data` to write to the symbol list.
- Adds a python test for the new behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@IvoDD IvoDD

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix symbol list bug on compaction (#1324) man-group/ArcticDB
2 participants
@IvoDD @alexowens90