-
Notifications
You must be signed in to change notification settings - Fork 81
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
Refactor indexer version config #332
base: main
Are you sure you want to change the base?
Changes from all commits
7de84f4
c7931c5
9fedd3e
f3d2e7f
41e489b
831fa71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,8 @@ pub struct IndexerGrpcProcessorConfig { | |
pub auth_token: String, | ||
// Version to start indexing from | ||
pub starting_version: Option<u64>, | ||
// Version to start indexing from if nothing in db | ||
pub starting_version_if_nothing_in_db: Option<u64>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we probably want a better name.. also, for these two parameters, it's not clear which one will take precedence. we probably should either use an enum here, or ensure they are mutually exclusive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we should avoid using enums to maintain backward compatibility. The current implementation works as follows:
|
||
// Version to end indexing at | ||
pub ending_version: Option<u64>, | ||
// Number of tasks waiting to pull transaction batches from the channel and process them | ||
|
@@ -77,6 +79,7 @@ impl RunnableConfig for IndexerGrpcProcessorConfig { | |
self.grpc_http2_config.clone(), | ||
self.auth_token.clone(), | ||
self.starting_version, | ||
self.starting_version_if_nothing_in_db, | ||
self.ending_version, | ||
self.number_concurrent_processing_tasks, | ||
self.db_pool_size, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,6 +55,7 @@ pub struct Worker { | |
pub grpc_http2_config: IndexerGrpcHttp2Config, | ||
pub auth_token: String, | ||
pub starting_version: Option<u64>, | ||
pub starting_version_if_nothing_in_db: Option<u64>, | ||
pub ending_version: Option<u64>, | ||
pub number_concurrent_processing_tasks: usize, | ||
pub gap_detection_batch_size: u64, | ||
|
@@ -73,6 +74,7 @@ impl Worker { | |
grpc_http2_config: IndexerGrpcHttp2Config, | ||
auth_token: String, | ||
starting_version: Option<u64>, | ||
starting_version_if_nothing_in_db: Option<u64>, | ||
ending_version: Option<u64>, | ||
number_concurrent_processing_tasks: Option<usize>, | ||
db_pool_size: Option<u32>, | ||
|
@@ -107,6 +109,7 @@ impl Worker { | |
indexer_grpc_data_service_address, | ||
grpc_http2_config, | ||
starting_version, | ||
starting_version_if_nothing_in_db, | ||
ending_version, | ||
auth_token, | ||
number_concurrent_processing_tasks, | ||
|
@@ -141,27 +144,37 @@ impl Worker { | |
"[Parser] Finished migrations" | ||
); | ||
|
||
let starting_version_from_db = self | ||
let mut starting_version_from_db = None; | ||
|
||
let mut starting_version = match self | ||
.get_start_version() | ||
.await | ||
.expect("[Parser] Database error when getting starting version") | ||
.unwrap_or_else(|| { | ||
.expect("[Parser] Database error when getting starting version") { | ||
None => { | ||
info!( | ||
processor_name = processor_name, | ||
service_type = PROCESSOR_SERVICE_TYPE, | ||
"[Parser] No starting version from db so starting from version 0" | ||
); | ||
0 | ||
}); | ||
self.starting_version_if_nothing_in_db.unwrap_or(0) | ||
} | ||
Some(version) => { | ||
starting_version_from_db = Some(version); | ||
version | ||
} | ||
}; | ||
|
||
let starting_version = self.starting_version.unwrap_or(starting_version_from_db); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for making the change! This line is still necessary to override with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! Fixed |
||
if let Some(force_start_version) = self.starting_version { | ||
starting_version = force_start_version | ||
} | ||
|
||
info!( | ||
processor_name = processor_name, | ||
service_type = PROCESSOR_SERVICE_TYPE, | ||
stream_address = self.indexer_grpc_data_service_address.to_string(), | ||
final_start_version = starting_version, | ||
start_version_from_config = self.starting_version, | ||
starting_version_if_nothing_in_db = self.starting_version_if_nothing_in_db, | ||
start_version_from_db = starting_version_from_db, | ||
"[Parser] Building processor", | ||
); | ||
|
@@ -746,4 +759,4 @@ pub fn build_processor( | |
UserTransactionProcessor::new(db_pool, per_table_chunk_sizes), | ||
), | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: update the document
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for being late on this.
a question about this: if
starting_version
is specific, does it also mean that processing start from this version regardless of db version?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a new field called "override_starting_version." The logic is straightforward: