apache / kvrocks

Apache Kvrocks is a distributed key value NoSQL database that uses RocksDB as storage engine and is compatible with Redis protocol.
https://kvrocks.apache.org/
Apache License 2.0
3.46k stars 447 forks source link

Proposal: Refactor source file structure #977

Closed PragmaTwice closed 1 year ago

PragmaTwice commented 1 year ago

Search before asking

Motivation

As mentioned in https://github.com/apache/incubator-kvrocks/pull/973#pullrequestreview-1137848493, we can categorize source files in to different folders to modularize the source structure.

Solution

The preliminary plan is as follows:

common/
  event_util fd_util parse_util cron encoding db_util event_util log_collector
  parse_util rw_lock status tls_util util

database/
  batch_extractor compact_filter compaction_checker event_listener geohash
  lock_manager redis_db redis_disk redis_metadata redis_pubsub
  scripting stats storage table_properties_collector

service/
  server worker redis_reply redis_connection redis_request

distributed/
  cluster redis_slot replication slot_import slot_migrate 

data_types/
  redis_bitmap redis_bitmap_string redis_geo redis_list redis_hash redis_set
  redis_sortedint redis_stream_base redis_stream redis_string redis_zset

commands/
  redis_cmd (need to be divided)

config/
  config config_type config_util

crypto/
  rand sha1 rocksdb_crc32c

Since I am not very familiar with certain parts of the code, I welcome everyone's thoughts, comments and suggestions!

cc @git-hulk @ShooterIT @caipengbo @tisonkun @torwig

Are you willing to submit a PR?

tisonkun commented 1 year ago

Will version.h.in leave as is? I'm thinking of moving VERSION file in the src folder so that we have a shorter top-level file list.

PragmaTwice commented 1 year ago

In my thought, these source folders are all in the src directory.

I am not sure where to put version.h.in now, maybe just in src/.

tanruixiang commented 1 year ago

I think the separation of redis_cmd can be done by referring to the official command classification of redis. https://redis.io/commands/

PragmaTwice commented 1 year ago

Hi everyone, how do you like this proposal?

I think the current source files are cluttered in one directory, and as the source files become more and more numerous, it becomes more and more inconvenient to manage & locate them in one directory.

If you like this, you can give your thoughts and opinions on the current file classification to avoid me misclassifying some source files.

git-hulk commented 1 year ago

@PragmaTwice I have some proposals about naming:

  1. distributed vs cluster - distributed looks too general and those files are cluster related
  2. data_types vs types - types is intuitive enough?
  3. database vs storage - I think storage is more precise than database
  4. service vs network

and seems a bit confused that putting crc32 and rand in crypto.

git-hulk commented 1 year ago

see whether folks have other suggestions.

PragmaTwice commented 1 year ago

@PragmaTwice I have some proposals about naming:

  1. distributed vs cluster - distributed looks too general and those files are cluster related
  2. data_types vs types - types is intuitive enough?
  3. database vs storage - I think storage is more precious than database
  4. service vs network

and seems a bit confuse that puts crc32 and rand in crypto.

Greate advices, I will follow them later.

caipengbo commented 1 year ago

@PragmaTwice I have a few ideas of my own:

PragmaTwice commented 1 year ago

Thanks. Here is the second edition: cc @git-hulk @caipengbo

common/
  event_util fd_util parse_util cron encoding db_util event_util log_collector
  parse_util rw_lock status tls_util util rand sha1 rocksdb_crc32c

storage/
  batch_extractor compact_filter compaction_checker event_listener
  lock_manager redis_db redis_disk redis_metadata redis_pubsub
  scripting stats storage table_properties_collector geohash

network/
  server worker redis_reply redis_connection redis_request

cluster/
  cluster redis_slot replication slot_import slot_migrate 

types/
  redis_bitmap redis_bitmap_string redis_geo redis_list redis_hash redis_set
  redis_sortedint redis_stream_base redis_stream redis_string redis_zset

commands/
  redis_cmd (need to be divided)

config/
  config config_type config_util

./
  version main

commands may be split up and placed under each data type?

Great idea, I think we can discuss this after this classification.

git-hulk commented 1 year ago

Looks good to me

caipengbo commented 1 year ago

LGTM

PragmaTwice commented 1 year ago

Thanks all. I will prepare a PR soon.