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

issue-1146: preliminary changes: extract some localdb methods to the interface, add stub for index cache #1478

Merged
merged 12 commits into from
Jul 5, 2024

Conversation

debnatkh
Copy link
Collaborator

@debnatkh debnatkh commented Jun 21, 2024

Currently RO transactions access TIndexTabletDatabase via TIndexTabletState methods like

TIndexTabletState::ReadNode(
    TIndexTabletDatabase& db,
    ...

which in turn calls do stuff like db.ReadNode(nodeId, commitId, node).

We want to cache some index data from the Index tables.

For this in-memory cache to implement the same set of methods, TIndexTabletDatabase is replaced with an interface IIndexTabletDatabase , which is implemented by TInMemoryIndexState (which, for now, is a stub). Its superset, IIndexTabletDatabase is also implemented by TIndexTabletDatabase.

To ensure in-memory cache validity, every RW transaction that modifies inode index records all modifying operations, and on completion of those transactions, these operations will be applied to the in-memory state. This is done using TIndexTabletDatabaseProxy : public TIndexTabletDatabase (to be implemented), which forwards all requests to the underlying TIndexTabletDatabase.

classDiagram
TNiceDb <|-- TIndexTabletDatabase
TIndexTabletDatabase : actual implementation that accesses LocalDB
    IIndexTabletDatabase <|-- TInMemoryIndexState
class TInMemoryIndexState {
- ApplyDelta(changes to inmemory state)
}

    IIndexTabletDatabase <|-- TIndexTabletDatabase
    class IIndexTabletDatabase {
      + bool ReadNode()
      + bool ReadNodeVer()
      + bool ReadNodeAttr_s()
      + bool ReadNodeAttrVer_s()
      + bool ReadNodeRef_s()
      + bool PrechargeNodeRefs()
      + bool ReadNodeRefVer_s()
    }

TIndexTabletDatabase<|-- TIndexTabletDatabaseProxy : forwards all DB accesses
TIndexTabletDatabaseProxy: records all modifications of the tables
TInMemoryIndexState<|.. TIndexTabletDatabaseProxy : can apply recorded changes\nto in-memory state

Loading

References #1146

@debnatkh debnatkh added the filestore Add this label to run only cloud/filestore build and tests on PR label Jun 21, 2024
@debnatkh debnatkh marked this pull request as ready for review June 21, 2024 14:55
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 3243440.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1563 1563 0 0 0 0

@debnatkh debnatkh marked this pull request as draft June 25, 2024 16:34
@debnatkh debnatkh marked this pull request as ready for review June 26, 2024 19:16
Copy link
Contributor

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo: some tests FAILED for commit 30aef9f.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1564 1562 0 2 0 0

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 9225ee4.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1563 1563 0 0 0 0

@debnatkh debnatkh force-pushed the users/debnatkh/issue-1146-extract-indexstate-iface branch from adf12c1 to 584851d Compare July 2, 2024 00:46
Copy link
Contributor

github-actions bot commented Jul 2, 2024

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 584851d.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1634 1634 0 0 0 0

Copy link
Contributor

github-actions bot commented Jul 4, 2024

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit b401a04.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1634 1634 0 0 0 0

Copy link
Contributor

github-actions bot commented Jul 4, 2024

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e7c00bc.

TESTS PASSED ERRORS FAILED SKIPPED MUTED?
1634 1634 0 0 0 0

@debnatkh debnatkh requested a review from qkrorlqr July 4, 2024 15:53
@debnatkh debnatkh requested a review from yegorskii July 5, 2024 09:50
@debnatkh debnatkh merged commit 91aac7f into main Jul 5, 2024
9 of 11 checks passed
@debnatkh debnatkh deleted the users/debnatkh/issue-1146-extract-indexstate-iface branch July 5, 2024 12:30
qkrorlqr pushed a commit that referenced this pull request Jul 6, 2024
…interface, add stub for index cache (#1478)

issue-1146: preliminary changes: extract some localdb methods to the interface, add stub for index cache
qkrorlqr added a commit that referenced this pull request Jul 6, 2024
…StageWrite EvPut type optimization (#1557)

* issue-1519: add some tests that `AddData` changes the inode size (#1525)

* issue-1350: we shouldn't fail the whole ListNodes request in case we somehow (due to bugs) lose some of the files in followers (#1526)

* issue-1350: XAttr ut and some request validation + uts (#1534)

* issue-1350: XAttr ut

* issue-1350: proper FollowerId validation in IndexTabletActor::CreateHandle

* issue-1350: validating ConfigureFollowers and ConfigureAsFollower requests

* issue-1146: preliminary changes: extract some localdb methods to the interface, add stub for index cache (#1478)

issue-1146: preliminary changes: extract some localdb methods to the interface, add stub for index cache

* issue-539: use NKikimrBlobStorage::UserData for three-stage write, not NKikimrBlobStorage::TabletLog (#1552)

* issue-1350: supported RenameNode for multitablet filesystems, fixed CreateHandle by Name for existing files - request should be sent to the follower specified in the Leader's response, not to the follower specified in the request (#1555)

* issue-1350: supported RenameNode for multitablet filesystems, fixed CreateHandle by Name for existing files - request should be sent to the follower specified in the Leader's response, not to the follower specified in the request

* issue-1350: deleted a check which is actually excessive

* issue-1350: unlinking a node in follower if this node was a RenameNode op target

* issue-1350: unlinking a node in follower if this node was a RenameNode op target - deleted unneeded include

* fixed build after merge

* updated CMakeLists.txt

---------

Co-authored-by: Maxim Deb Natkh <debnatkh@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filestore Add this label to run only cloud/filestore build and tests on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants