Skip to content

add input_static_file plugin && support onetime config #2063

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

Open
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

henryzhx8
Copy link
Collaborator

No description provided.

if (value.isNull()) {
hash ^= std::hash<std::string>{}("null");
} else if (value.isBool()) {
hash ^= std::hash<bool>{}(value.asBool());
Copy link
Collaborator

Choose a reason for hiding this comment

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

因为这里实在罗列所有类型,后面如果新增一个,如何类型,如何保证这里不漏改?

例如GetOptionalUInt64Param就是这次新加的

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这里是穷举了所有json支持的类型,除非lib新增了支持的类型,否则这里是不可能新增类型的。
Getxxx函数是有需要就增加的原则,所以一开始就没有要穷举

@henryzhx8 henryzhx8 added the feature New feature label Feb 11, 2025
#include "config/PipelineConfig.h"

#include "common/JsonUtil.h"
#include "config/OnetimeConfigManager.h"
Copy link
Collaborator

@yyuuttaaoo yyuuttaaoo Apr 27, 2025

Choose a reason for hiding this comment

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

这个依赖关系合不合理?OnetimeConfigStatus需要用是不是把定义挪过来更合适

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OnetimeConfigStatus本身在OnetimeConfigInfoManager里用,这里也要用,这个是cpp文件,使用定义是合理的

return false;
}

bool FileDiscoveryOptions::IsFilepathInBlacklist(const std::string& filepath) const {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这些方法本来没有吗?那原来的文件黑名单是怎么实现的

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

之前有个IsObjectInBlacklist,它里面不止有文件黑名单,比较乱

@henryzhx8 henryzhx8 requested a review from Copilot April 28, 2025 07:51
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for an input_static_file plugin as well as onetime config support, while removing the old adhoc checkpoint mechanism. Key changes include:

  • Refactoring plugin registration methods to distinguish between one‑time and continuous input plugins
  • Updates to CollectionPipeline and Application logic to handle onetime configuration lifecycles
  • Removal of legacy adhoc checkpoint files and related functionality

Reviewed Changes

Copilot reviewed 121 out of 122 changed files in this pull request and generated no comments.

Show a summary per file
File Description
core/collection_pipeline/queue/ExactlyOnceSenderQueue.h Updated include path for RangeCheckpoint
core/collection_pipeline/queue/ExactlyOnceQueueManager.h Updated include path for RangeCheckpoint
core/collection_pipeline/plugin/instance/ProcessorInstance.h Added friend declaration for InputStaticFileUnittest
core/collection_pipeline/plugin/PluginRegistry.{h,cpp} Modified input creator methods and PluginCat enum to support onetime and continuous inputs
core/collection_pipeline/CollectionPipeline{.h, .cpp} Added onetime config state handling in pipeline initialization and stop methods
core/collection_pipeline/CollectionPipelineManager.{h,cpp} Introduced ClearInputUnusedCheckpoints method
core/collection_pipeline/CollectionPipelineContext.h Added onetime pipeline running flag in context
core/application/Application.cpp Updated timing and checkpoint calls to support onetime config check/dump and removal
core/app_config/AppConfig.{h,cpp} Removed legacy adhoc checkpoint path and related code
(Multiple checkpoint files) Legacy adhoc checkpoint files removed
Files not reviewed (1)
  • core/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (2)

core/collection_pipeline/plugin/PluginRegistry.h:61

  • [nitpick] The new PluginCat enum values differentiate one‑time and continuous input plugins; consider adding a brief comment for each value to clarify their intended use.
enum PluginCat { CONTINUOUS_INPUT_PLUGIN, ONETIME_INPUT_PLUGIN, PROCESSOR_PLUGIN, FLUSHER_PLUGIN };

core/application/Application.cpp:272

  • [nitpick] Consider breaking up this multi-variable declaration into separate lines or adding inline comments for clarity on each timer's purpose.
time_t curTime = 0, lastOnetimeConfigTimeoutCheckTime = 0, lastConfigCheckTime = 0, lastUpdateMetricTime = 0, lastCheckTagsTime = 0, lastQueueGCTime = 0, lastCheckUnusedCheckpointsTime = 0;

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for one‑time configurations through a new input_static_file plugin while refactoring the native input plugin registration and lifecycle management. Key changes include:

  • Splitting native input plugins into continuous and one‑time types.
  • Updating pipeline initialization and shutdown to handle one‑time configuration lifecycles.
  • Removing legacy adhoc checkpoint code in favor of the new OnetimeConfigInfoManager.

Reviewed Changes

Copilot reviewed 121 out of 122 changed files in this pull request and generated no comments.

Show a summary per file
File Description
core/collection_pipeline/queue/ExactlyOnceSenderQueue.h Updated include path for RangeCheckpoint.
core/collection_pipeline/queue/ExactlyOnceQueueManager.h Updated include path and native input validation signature.
core/collection_pipeline/plugin/*.h/.cpp Modified input plugin creation, registration, and added unit test friend declarations.
core/collection_pipeline/CollectionPipeline*.h/.cpp Integrated one‑time config logic into pipeline initialization and stop methods.
core/application/Application.cpp Updated main application loop to load and manage one‑time config checkpoints.
core/app_config/AppConfig*.h/.cpp Removed legacy adhoc checkpoint directory references.
core/checkpoint/* Removed obsolete adhoc checkpoint related files.
Files not reviewed (1)
  • core/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (4)

core/collection_pipeline/plugin/PluginRegistry.cpp:103

  • [nitpick] Consider renaming the parameter 'isOnetime' to 'oneTime' if it aligns better with your project's naming conventions.
PluginRegistry::CreateInput(const string& name, bool isOnetime, const PluginInstance::PluginMeta& pluginMeta)

core/collection_pipeline/CollectionPipeline.cpp:480

  • Verify that the onetime configuration removal logic in the Stop method correctly handles cases where pipelines may be updated or removed concurrently.
if (mIsOnetime && isRemoving) { OnetimeConfigInfoManager::GetInstance()->RemoveConfig(mName); }

core/collection_pipeline/CollectionPipelineManager.cpp:173

  • Please add unit tests to verify the behavior of ClearInputUnusedCheckpoints to ensure that unused checkpoints are properly cleared.
void CollectionPipelineManager::ClearInputUnusedCheckpoints() {

core/application/Application.cpp:283

  • Ensure that the deletion of onetime configuration files is synchronized with pipeline updates to avoid potential race conditions.
if (curTime - lastOnetimeConfigTimeoutCheckTime >= 10) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants