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

Fixed unintentionally finished of the page output after the first file was processed. #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

t3t5u
Copy link

@t3t5u t3t5u commented Sep 6, 2024

Problem Overview

When the input is multiple files, the second and subsequent files are not output.

Cause of the Problem

Calling the PageBuilder#finish will invoke the PageOutput#finish.

https://github.com/embulk/embulk/blob/v0.11.4/embulk-core/src/main/java/org/embulk/spi/PageBuilderImpl.java#L255-L258

    public void finish() {
        doFlush();
        output.finish();
    }

Once the PageOutput#finish is invoked, the outputWorkers will be done and PageOutput#add will no longer work.

https://github.com/embulk/embulk/blob/v0.11.4/embulk-core/src/main/java/org/embulk/exec/LocalExecutorPlugin.java#L456-L463

        public void finish() {
            completeWorkers();
            for (int i = 0; i < scatterCount; i++) {
                if (filtereds[i] != null) {
                    filtereds[i].finish();
                }
            }
        }

https://github.com/embulk/embulk/blob/v0.11.4/embulk-core/src/main/java/org/embulk/exec/LocalExecutorPlugin.java#L444-L454

        public void add(Page page) {
            OutputWorker worker = outputWorkers[(int) (pageCount % scatterCount)];
            if (worker != null) {
                try {
                    worker.add(page);
                } catch (InterruptedException ex) {
                    throw new RuntimeException(ex);
                }
            }
            pageCount++;
        }

Therefore, the second and subsequent files will not be output.

@t3t5u
Copy link
Author

t3t5u commented Sep 6, 2024

@hishidama
Please review. 🙇

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

Successfully merging this pull request may close these issues.

1 participant