Commit 4fddca7
authored
Fix dense fragment domains during global order write with maximum fragment size (#5655)
Fixes CORE-290.
## Report
In CORE-290 a customer reported issues with corrupt arrays after running
consolidation. The symptom was memory allocation errors when opening an
array. The root cause turned out to be the consolidation itself was
writing new fragments where the fragment domain did not match the number
of tiles in the fragment.
## Root cause analysis
Stepping through a minimal reproducer revealed that consolidation is not
at fault at all - instead it is the global order writer
`max_fragment_size_` field. This field, presumably meant to be used for
consolidation only but valid for other writes, instructs the writer to
write multiple fragments each under the requested size as necessary.
When a write splits its data into multiple fragments this way, nothing
need be done for sparse arrays. But dense fragment metadata relies on
the domain to determine the number of tiles, and the global order writer
did not update the fragment metadata domain when splitting its write
into multiple fragments.
This pull request fixes that issue by ensuring that the global order
writer updates the fragment metadata domain to reflect what was actually
written into that fragment.
This is easier said than done. In CORE-290 I offered four ways we can
fix this. The chosen solution:
## Design
The fragment metadata domain is a bounding rectangle. This means that
the global order writer must split the tiles of its input into fragments
at tile boundaries which bisect the bounding rectangle into two smaller
bounding rectangles.
To do so, we add a first pass `identify_fragment_tile_boundaries` which
returns a list of tile offsets where new fragments will begin. Upon
finishing a fragment, we use that tile offset to determine which
rectangle within the target subarray the fragment actually represents,
and update the fragment metadata accordingly. We use new functions
`is_rectangular_domain` to determine whether a `(start_tile, num_tiles)`
pair identifies a rectangle, and `domain_tile_offset` to compute that
rectangle.
Much of the complexity comes from the usage of the global order writer
which does happen in consolidation: multi-part writes. A user (or a
consolidation operation) can set a domain `D` which it intends to write
into, and then actually fill in all of the cells over multiple `submit`
calls which stream in the cells to write. It is not required for these
cells to be tile aligned. Because of that, and the need to write
rectangle fragments, a single `submit` cannot always determine whether a
tail of tiles belongs to its current fragment or must be deferred to the
next. To get around this we keep those tiles in memory in the
`global_write_state_` and prepend them to the user input in the next
`submit`.
## Testing
We add unit tests for `is_rectangular_domain` and `domain_tile_offset`,
including both example tests as well as rapidcheck properties to assert
general claims.
We add tests for the global order writer to make broad claims about what
the writer is supposed to do with respect to the `max_fragment_size`
parameter. We add examples and a rapidcheck test to exercise these
claims. In particular:
- no fragment should be created larger than this size
- two adjacent fragments must exceed that size (otherwise they could be
one fragment)
- fragments correctly populate the domain of the write query
Since the original report originated from consolidation, we also add
some consolidation tests **(TODO)** mimicking the customer input.
## TODO
- [x] consolidation test which mimics the customer report
- [x] test the scenario in the `FIXME` comment in
`global_order_writer.cc`
- [x] test with variable-length attributes
- [x] more careful test of `last_tiles_` buffering which observes the
amount of data buffered for a 3D write
---
TYPE: BUG
DESC: Fix dense global order writer use of `max_fragment_size_`1 parent a0704cf commit 4fddca7
File tree
29 files changed
+4436
-536
lines changed- test
- src
- support
- rapidcheck
- show
- src
- tiledb
- common
- sm
- fragment
- query
- writers
- tile
- test
- type/range
29 files changed
+4436
-536
lines changedLarge diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3713 | 3713 | | |
3714 | 3714 | | |
3715 | 3715 | | |
3716 | | - | |
3717 | | - | |
3718 | | - | |
3719 | | - | |
3720 | | - | |
3721 | | - | |
3722 | | - | |
3723 | | - | |
3724 | | - | |
3725 | | - | |
3726 | | - | |
3727 | | - | |
3728 | | - | |
| 3716 | + | |
3729 | 3717 | | |
3730 | 3718 | | |
3731 | 3719 | | |
| |||
3735 | 3723 | | |
3736 | 3724 | | |
3737 | 3725 | | |
3738 | | - | |
3739 | | - | |
3740 | | - | |
3741 | | - | |
3742 | | - | |
3743 | | - | |
3744 | | - | |
3745 | | - | |
3746 | | - | |
| 3726 | + | |
3747 | 3727 | | |
3748 | 3728 | | |
3749 | 3729 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
36 | 36 | | |
37 | 37 | | |
38 | 38 | | |
39 | | - | |
| 39 | + | |
| 40 | + | |
40 | 41 | | |
41 | 42 | | |
42 | 43 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
0 commit comments