Skip to content

Conversation

@keith-turner
Copy link
Contributor

This commit modifies the compaction coordinator to reliably refresh hosted tablets in memory metadata after a compaction. The RPC request to a hosted tablet asking it to refresh happens after updating a tablets metadata for a compaction. The changes in the PR ensure that if the manager process dies after making the metadata update but before it makes the RPC that this will happen when the manager restarts.

The implementation for this relies on a new ~refresh sections in the metadata table. System compactions on hosted tablets will write a ~refresh entry before trying to update a tablets metadata. This entry allows the compaction commit process to be fault tolerant.

Javadoc was added to CompactionCoordinator.compactionCompleted() describing the overall process of committing a compaction.

This commit also refactored the refresh RPC code in bulk import so that it could also be used by compactions.

This commit also made the tablet metadata update for a compaction add scan entries for removed files when a tablet is hosted. This was done to prevent the Accumulo GC from removing compacted files that may be in use by an immediate scan.

fixes #3462

This commit modifies the compaction coordinator to reliably refresh
hosted tablets in memory metadata after a compaction.  The RPC request
to a hosted tablet asking it to refresh happens after updating a tablets
metadata for a compaction. The changes in the PR ensure that if the
manager process dies after making the metadata update but before it
makes the RPC that this will happen when the manager restarts.

The implementation for this relies on a new ~refresh sections in the
metadata table.  System compactions on hosted tablets will write a
~refresh entry before trying to update a tablets metadata.  This entry
allows the compaction commit process to be fault tolerant.

Javadoc was added to CompactionCoordinator.compactionCompleted()
describing the overall process of committing a compaction.

This commit also refactored the refresh RPC code in bulk import so that
it could also be used by compactions.

This commit also made the tablet metadata update for a compaction add
scan entries for removed files when a tablet is hosted.  This was done
to prevent the Accumulo GC from removing compacted files that may be in
use by an immediate scan.

fixes apache#3462
@dlmarion
Copy link
Contributor

dlmarion commented Jul 3, 2023

Curious if you have looked at #3200 and your thoughts on using ZK to signal Tablet metadata changes. #3200 contains a Cache, but that could be removed and it could notify listeners instead.

@keith-turner
Copy link
Contributor Author

keith-turner commented Jul 3, 2023

Curious if you have looked at #3200 and your thoughts on using ZK to signal Tablet metadata changes.

ZK is not very scalable in terms of writes (I think each write goes to the leader which then forwards it to all other ZK servers and waits for a majority to respond). This potential write bottleneck seem like an issue when considering it as an option for signaling tablet metadata refresh.

As for the tablet metadata update in the tablet server I have not had a chance to look into it in detail. Some of the issues that I know will arise for refresh off the top of my head are :

  • Needs to properly synchronize the cut over from in memory map to minor compacted file w.r.t. initiating new scan. A new scan should always use one or the other, never both or neither.
  • Needs to properly synchronize the tracking of scan file entries in tablet metadata and active scans.
  • Needs to properly synchronize with write ahead log recovery and tablets update around that.
  • Needs to properly synchronize with a tablet that is loading or unloading.

The tablet server code has synchronization around bulk import, compactions, and splits that can be remove. I am thinking after we remove that code we can evaluate what needs are left and look into simplifying the tablets synchronization model that refresh will have to fit into.

Copy link
Member

@DomGarguilo DomGarguilo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far from what I can tell. Just a few minor comments/suggestions.

@keith-turner keith-turner linked an issue Jul 3, 2023 that may be closed by this pull request
keith-turner and others added 2 commits July 5, 2023 11:26
…ionIT.java

Co-authored-by: Dave Marion <dlmarion@apache.org>
@keith-turner keith-turner merged commit 8308192 into apache:elasticity Jul 5, 2023
@keith-turner keith-turner deleted the accumulo-3462-2 branch July 5, 2023 19:51
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Handle faults and race conditions in compaction commit

4 participants